fix(pagination): drive paginate by cursor metadata instead of re-fetching the first page (#561)

paginate() rebuilt a fresh { limit } query on every loop iteration and
never read cursor metadata, so any page containing exactly limit items
was re-requested forever, yielding duplicate items. The sibling helpers
parseCursorPage/buildPaginationQuery were never used by paginate.

- paginate() now accepts a fetch that returns CursorPage<T> and feeds
  nextCursor back through buildPaginationQuery for the next request
- stops when the cursor is null/empty, a page is shorter than limit,
  or maxPages is reached (hasMore=false is also honored)
- rewrote the paginate test block: cursor advancement, no-duplicate
  guarantee for a full page, maxPages, empty page, short page, and
  hasMore=false stop conditions (16 tests, all passing)

Co-authored-by: foxxx009 <foxxx009@users.noreply.github.com>

Author: foxxx009

src/pagination.ts

@@ -38,40 +38,43 @@ export function buildPaginationQuery(
 /**
  * Async iterator helper that auto-paginates through a cursor-based list endpoint.
  *
+ * `fetchPage` receives a `PaginationQuery` (containing the `cursor` from the
+ * previous response when present) and must return a `CursorPage<T>` describing
+ * the fetched page: its items, the cursor for the next page, and whether more
+ * pages exist. Iteration stops when the returned cursor is null/empty, a page
+ * is shorter than `limit` (when `limit` is set), or `maxPages` is reached.
+ *
  * @example
  * for await (const agent of paginate(client.agents.list.bind(client.agents))) {
  *   console.log(agent.id);
  * }
  */
 export async function* paginate<T>(
-  fetchPage: (query?: PaginationQuery) => Promise<readonly T[]>,
+  fetchPage: (query?: PaginationQuery) => Promise<CursorPage<T>>,
   options?: { limit?: number; maxPages?: number },
 ): AsyncGenerator<T, void, unknown> {
   const maxPages = options?.maxPages ?? 100;
-  let pageCount = 0;
+  let nextCursor: string | undefined;
+
+  for (let pageCount = 0; pageCount < maxPages; pageCount += 1) {
+    const query: PaginationQuery = {
+      ...(options?.limit !== undefined ? { limit: options.limit } : {}),
+      ...buildPaginationQuery(nextCursor),
+    };
+    const page = await fetchPage(query);
 
-  while (pageCount < maxPages) {
-    const query: PaginationQuery = options?.limit
-      ? { limit: options.limit }
-      : {};
-    const items = await fetchPage(query);
-    for (const item of items) {
+    for (const item of page.items) {
       yield item;
     }
-    // Without a cursor mechanism from the response, we stop after one page
-    // since we can't know if there are more items.
-    pageCount += 1;
-    // If we got fewer items than the limit, we're done
-    if (options?.limit && items.length < options.limit) {
-      break;
-    }
-    // Without response headers exposing next cursor, we stop to avoid infinite loop
-    if (items.length === 0) {
+
+    // A short page means the dataset is exhausted even if a cursor leaks back.
+    if (options?.limit !== undefined && page.items.length < options.limit) {
       break;
     }
-    // If no limit specified, we do one page (can't know if there are more)
-    if (!options?.limit) {
+    // No further cursor: the last page was reached.
+    if (!page.hasMore || page.nextCursor === null || page.nextCursor === '') {
       break;
     }
+    nextCursor = page.nextCursor;
   }
 }

tests/pagination-helper.test.ts

@@ -4,6 +4,7 @@ import {
   buildPaginationQuery,
   paginate,
 } from '../src/pagination';
+import type { CursorPage } from '../src/pagination';
 
 describe('pagination helper (issue #61)', () => {
   describe('parseCursorPage', () => {
@@ -57,8 +58,17 @@ describe('pagination helper (issue #61)', () => {
   });
 
   describe('paginate', () => {
-    it('yields all items from a single page', async () => {
-      const fetchPage = vi.fn().mockResolvedValue([1, 2, 3]);
+    const page = (
+      items: number[],
+      nextCursor: string | null,
+    ): CursorPage<number> => ({
+      items,
+      nextCursor,
+      hasMore: nextCursor != null && nextCursor !== '',
+    });
+
+    it('yields all items from a single cursor-less page', async () => {
+      const fetchPage = vi.fn().mockResolvedValue(page([1, 2, 3], null));
       const results: number[] = [];
       for await (const item of paginate<number>(fetchPage)) {
         results.push(item);
@@ -67,24 +77,72 @@ describe('pagination helper (issue #61)', () => {
       expect(fetchPage).toHaveBeenCalledTimes(1);
     });
 
-    it('stops at maxPages when limit is set', async () => {
-      const fetchPage = vi.fn().mockResolvedValue([1, 2]);
+    it('advances on cursor metadata across a two-page dataset (issue #403)', async () => {
+      const fetchPage = vi
+        .fn()
+        .mockResolvedValueOnce(page([1, 2], 'c1'))
+        .mockResolvedValueOnce(page([3, 4], null));
+      const results: number[] = [];
+      for await (const item of paginate<number>(fetchPage, { limit: 2 })) {
+        results.push(item);
+      }
+      expect(results).toEqual([1, 2, 3, 4]);
+      expect(fetchPage).toHaveBeenCalledTimes(2);
+      // First call carries only the limit, second call feeds the cursor back.
+      expect(fetchPage).toHaveBeenNthCalledWith(1, { limit: 2 });
+      expect(fetchPage).toHaveBeenNthCalledWith(2, {
+        limit: 2,
+        cursor: 'c1',
+      });
+    });
+
+    it('never re-fetches a page whose length equals limit (issue #403)', async () => {
+      // Old bug: a full page with no further cursor was re-requested forever,
+      // yielding duplicate items.
+      const fetchPage = vi
+        .fn()
+        .mockResolvedValueOnce(page([1, 2], null))
+        .mockResolvedValue(page([1, 2], null));
+      const results: number[] = [];
+      for await (const item of paginate<number>(fetchPage, { limit: 2 })) {
+        results.push(item);
+      }
+      expect(results).toEqual([1, 2]);
+      expect(fetchPage).toHaveBeenCalledTimes(1);
+    });
+
+    it('stops at maxPages while the server keeps returning cursors', async () => {
+      const fetchPage = vi.fn().mockResolvedValue(page([1, 2], 'c-next'));
       const results: number[] = [];
       for await (const item of paginate<number>(fetchPage, {
         limit: 2,
         maxPages: 3,
       })) {
         results.push(item);
       }
-      expect(results.length).toBe(6); // 3 pages * 2 items
+      expect(results).toEqual([1, 2, 1, 2, 1, 2]);
       expect(fetchPage).toHaveBeenCalledTimes(3);
+      expect(fetchPage).toHaveBeenLastCalledWith({
+        limit: 2,
+        cursor: 'c-next',
+      });
+    });
+
+    it('stops on an empty page', async () => {
+      const fetchPage = vi.fn().mockResolvedValue(page([], null));
+      const results: number[] = [];
+      for await (const item of paginate<number>(fetchPage, { limit: 10 })) {
+        results.push(item);
+      }
+      expect(results).toEqual([]);
+      expect(fetchPage).toHaveBeenCalledTimes(1);
     });
 
-    it('stops when page returns fewer items than limit', async () => {
+    it('stops when a page is shorter than the limit', async () => {
       const fetchPage = vi
         .fn()
-        .mockResolvedValueOnce([1, 2])
-        .mockResolvedValueOnce([3]);
+        .mockResolvedValueOnce(page([1, 2], 'c1'))
+        .mockResolvedValueOnce(page([3], 'c2'));
       const results: number[] = [];
       for await (const item of paginate<number>(fetchPage, { limit: 2 })) {
         results.push(item);
@@ -93,13 +151,17 @@ describe('pagination helper (issue #61)', () => {
       expect(fetchPage).toHaveBeenCalledTimes(2);
     });
 
-    it('stops on empty page', async () => {
-      const fetchPage = vi.fn().mockResolvedValue([]);
+    it('stops when hasMore is false even if a cursor leaks back', async () => {
+      const fetchPage = vi.fn().mockResolvedValue({
+        items: [1],
+        nextCursor: 'c1',
+        hasMore: false,
+      });
       const results: number[] = [];
-      for await (const item of paginate<number>(fetchPage, { limit: 10 })) {
+      for await (const item of paginate<number>(fetchPage)) {
         results.push(item);
       }
-      expect(results).toEqual([]);
+      expect(results).toEqual([1]);
       expect(fetchPage).toHaveBeenCalledTimes(1);
     });
   });