fix(agent-sdk): stop maxItems from dropping rows and reporting a stale cursor

.changeset/agent-sdk-max-items-row-loss.md

@@ -0,0 +1,8 @@
+---
+"@rozenite/agent-sdk": patch
+"@rozenite/agent-shared": minor
+---
+
+Fix `autoPaginate.maxItems` silently dropping rows and reporting a `nextCursor` that skipped past them. The auto-pagination helper now requests exactly the rows still needed on every call (including the first), so a well-behaved tool's own cursor stays correct by construction instead of pointing past rows the caller never received.
+
+For tools that clamp or ignore the requested `limit` and still return more rows than asked, the extra rows are trimmed and the page's `nextCursor` is dropped rather than propagated (it can no longer be trusted to resume correctly). This is now signalled explicitly via a new `truncated: true` field on `PageEnvelope` (`@rozenite/agent-shared`), paired with `hasMore: true`, meaning "rows were dropped; resume position is unknown - restart pagination or narrow the query."

packages/agent-sdk/src/__tests__/pagination.test.ts

@@ -1,6 +1,50 @@
 import { describe, expect, it, vi } from 'vitest';
 import { callToolWithOptionalPagination } from '../pagination.js';
 
+// Mirrors the real cursor encoding used by production paginated tools
+// (e.g. packages/middleware/src/agent/local-domains.ts `paginateRows`):
+// base64url JSON of { v: 1, scope, index }.
+const encodeCursor = (scope: string, index: number): string => {
+  return Buffer.from(
+    JSON.stringify({ v: 1, scope, index }),
+    'utf8',
+  ).toString('base64url');
+};
+
+const decodeCursorIndex = (cursor: string): number => {
+  const decoded = JSON.parse(
+    Buffer.from(cursor, 'base64url').toString('utf8'),
+  ) as { index: number };
+  return decoded.index;
+};
+
+// A well-behaved paginated tool: honors whatever `limit` it's asked for on
+// each call, and encodes `nextCursor` as the exact index to resume from.
+// Used to prove the row-loss is caused by the SDK's own request/merge
+// logic, not by a misbehaving producer.
+const createWellBehavedPaginatedTool = (
+  scope: string,
+  totalRows: number,
+) => {
+  const rows = Array.from({ length: totalRows }, (_, i) => ({ id: i }));
+  return vi.fn().mockImplementation((_name: string, args: unknown) => {
+    const record = (args ?? {}) as { limit?: number; cursor?: string };
+    const limit = record.limit ?? totalRows;
+    const startIndex = record.cursor ? decodeCursorIndex(record.cursor) : 0;
+    const endIndex = Math.min(startIndex + limit, rows.length);
+    const items = rows.slice(startIndex, endIndex);
+    const hasMore = endIndex < rows.length;
+    return Promise.resolve({
+      items,
+      page: {
+        limit,
+        hasMore,
+        nextCursor: hasMore ? encodeCursor(scope, endIndex) : undefined,
+      },
+    });
+  });
+};
+
 describe('agent tool pagination helper', () => {
   it('returns raw result when auto-pagination is not requested', async () => {
     const client = {
@@ -78,6 +122,11 @@ describe('agent tool pagination helper', () => {
   });
 
   it('caps an oversized first page without fetching another page', async () => {
+    // This producer ignores the requested `limit` (it always returns 3
+    // items no matter what's asked) - the guaranteed-correct fallback path.
+    // Since it over-returns relative to what was requested (2, driven by
+    // maxItems), its `nextCursor: 'c1'` cannot be trusted to resume after
+    // the item we discard, so it must be dropped and `truncated` reported.
     const client = {
       callTool: vi.fn().mockResolvedValue({
         items: [{ id: 1 }, { id: 2 }, { id: 3 }],
@@ -92,16 +141,27 @@ describe('agent tool pagination helper', () => {
       { pagesLimit: 2, maxItems: 2 },
     )) as {
       items: Array<{ id: number }>;
-      page: { hasMore: boolean; nextCursor?: string; limit: number };
+      page: {
+        hasMore: boolean;
+        nextCursor?: string;
+        limit: number;
+        truncated?: boolean;
+      };
     };
 
     expect(result.items.map((item) => item.id)).toEqual([1, 2]);
     expect(result.page).toEqual({
       limit: 3,
       hasMore: true,
-      nextCursor: 'c1',
+      nextCursor: undefined,
+      truncated: true,
     });
     expect(client.callTool).toHaveBeenCalledTimes(1);
+    // The SDK asked for exactly the remaining budget (min(callerLimit=3,
+    // maxItems=2) = 2), not the caller's full page size of 3.
+    expect(client.callTool).toHaveBeenCalledWith('listRequests', {
+      limit: 2,
+    });
   });
 
   it('stops merging if a subsequent page is not paged', async () => {
@@ -202,6 +262,93 @@ describe('agent tool pagination helper', () => {
     expect(client.callTool).toHaveBeenCalledTimes(2);
   });
 
+  it('BUG REPRO: drops rows on the initial page because the SDK requests a full page instead of maxItems', async () => {
+    // A well-behaved 20-row producer. The caller asks for pages of 20 with
+    // maxItems: 5. The un-fixed SDK forwards `limit: 20` unmodified on the
+    // very first call, receives the ENTIRE 20-row dataset in one page
+    // (hasMore: false - there is nothing left upstream), then trims the
+    // response down to 5 items while keeping that page's `hasMore: false`.
+    // Rows 5-19 are real, were served by the producer, and are now
+    // unreachable: there's no cursor (hasMore is false) to go get them.
+    const tool = createWellBehavedPaginatedTool('initial-repro', 20);
+    const client = { callTool: tool };
+
+    const result = (await callToolWithOptionalPagination(
+      client,
+      'listRequests',
+      { limit: 20 },
+      { pagesLimit: 1, maxItems: 5 },
+    )) as {
+      items: Array<{ id: number }>;
+      page: { hasMore: boolean; nextCursor?: string; limit: number };
+    };
+
+    const receivedIds = result.items.map((item) => item.id);
+    const reachableViaCursor: number[] = [];
+    if (result.page.nextCursor) {
+      for (
+        let i = decodeCursorIndex(result.page.nextCursor);
+        i < 20;
+        i++
+      ) {
+        reachableViaCursor.push(i);
+      }
+    } else if (result.page.hasMore) {
+      // hasMore is true but there's no cursor to act on - still unreachable.
+    }
+    const union = new Set([...receivedIds, ...reachableViaCursor]);
+
+    const lostRows = Array.from({ length: 20 }, (_, i) => i).filter(
+      (id) => !union.has(id),
+    );
+    // Rows 5-19 were served by the producer but are absent from both what
+    // the caller received AND what's reachable via the reported cursor.
+    expect(lostRows).toEqual([]);
+    // The reported state must not falsely claim the dataset is exhausted.
+    expect(result.page.hasMore).toBe(true);
+  });
+
+  it('BUG REPRO: drops rows mid-loop because subsequent requests reuse the caller limit instead of the remaining budget', async () => {
+    // A well-behaved 20-row producer. Caller asks for pages of 5 with
+    // maxItems: 7 over up to 2 pages. Page 1 delivers rows 0-4 (5 items,
+    // remaining budget now 2). The un-fixed SDK's second call still asks
+    // for `limit: 5` (the caller's original page size) instead of the
+    // remaining budget of 2, so the producer legitimately returns rows
+    // 5-9 with a valid nextCursor pointing at row 10. The SDK keeps only
+    // 2 of those 5 rows (5, 6) to respect maxItems, discards rows 7-9, but
+    // still reports the producer's nextCursor - which skips straight over
+    // rows 7-9 to row 10. They are gone even though hasMore correctly
+    // (but misleadingly, given the gap) says there's more.
+    const tool = createWellBehavedPaginatedTool('mid-loop-repro', 20);
+    const client = { callTool: tool };
+
+    const result = (await callToolWithOptionalPagination(
+      client,
+      'listRequests',
+      { limit: 5 },
+      { pagesLimit: 2, maxItems: 7 },
+    )) as {
+      items: Array<{ id: number }>;
+      page: { hasMore: boolean; nextCursor?: string; limit: number };
+    };
+
+    const receivedIds = result.items.map((item) => item.id);
+    const reachableFromCursor = result.page.nextCursor
+      ? decodeCursorIndex(result.page.nextCursor)
+      : undefined;
+
+    // Rows 7, 8, 9 were served by the producer, are not in the caller's
+    // result, and are skipped over by the reported nextCursor (which jumps
+    // straight to row 10): silent data loss with a misleading resume point.
+    const droppedButSkippedOver = [7, 8, 9].filter(
+      (id) =>
+        !receivedIds.includes(id) &&
+        reachableFromCursor !== undefined &&
+        id < reachableFromCursor,
+    );
+    expect(droppedButSkippedOver).toEqual([]);
+  });
+
   it('requires pagesLimit when maxItems is used', async () => {
     const client = {
       callTool: vi.fn(),

packages/agent-sdk/src/pagination.ts

@@ -1,3 +1,4 @@
+import type { PageEnvelope } from '@rozenite/agent-shared';
 import type { AgentCallToolAutoPaginationOptions } from './types.js';
 
 type ToolCaller = {
@@ -6,12 +7,7 @@ type ToolCaller = {
 
 type PagedResponse = {
   items: unknown[];
-  page: {
-    limit: number;
-    hasMore: boolean;
-    nextCursor?: string;
-    reset?: boolean;
-  };
+  page: PageEnvelope;
   [key: string]: unknown;
 };
 
@@ -74,6 +70,73 @@ const getTrimmedItems = (
   return maxItems === undefined ? [...items] : items.slice(0, maxItems);
 };
 
+// `limit` is a universal convention across every paginated agent tool (see
+// packages/middleware/src/agent/local-domains.ts and the plugin
+// packages/*/src/shared/agent-tools.ts contracts), so it's safe for the SDK
+// to read and rewrite it when driving auto-pagination.
+const getCallerLimit = (args: Record<string, unknown>): number | undefined => {
+  return isPositiveInteger(args.limit) ? args.limit : undefined;
+};
+
+// The request limit for a call that must not fetch more than `remaining`
+// rows. Respects a caller-supplied `limit` as a ceiling when present;
+// otherwise requests exactly what's still needed.
+const getRequestLimit = (
+  callerLimit: number | undefined,
+  remaining: number,
+): number => {
+  return callerLimit === undefined ? remaining : Math.min(callerLimit, remaining);
+};
+
+/**
+ * Guaranteed-correct fallback for producers that clamp or ignore the
+ * requested `limit` (e.g. a tool that declares "Page size. Default 50, max
+ * 200" and returns its own default regardless of what's asked). If a
+ * response comes back with MORE rows than were requested, the rows beyond
+ * `requestedLimit` must be trimmed - but the producer's own `nextCursor`
+ * was computed assuming the caller received the whole page it sent, so it
+ * cannot be trusted to resume after the point we're truncating at.
+ *
+ * We only need to intervene when trimming would otherwise silently discard
+ * a resumable position: if the untouched response has no cursor to lose
+ * (hasMore is false / nextCursor is absent), trimming to the remaining
+ * budget is the same "reached maxItems, stop here" behavior this helper
+ * already performs elsewhere, and is left alone.
+ */
+const applyRequestBudget = (
+  response: PagedResponse,
+  requestedLimit: number | undefined,
+): PagedResponse => {
+  if (
+    requestedLimit === undefined ||
+    response.items.length <= requestedLimit
+  ) {
+    return response;
+  }
+
+  const trimmedItems = response.items.slice(0, requestedLimit);
+  const hadResumablePosition =
+    response.page.hasMore || response.page.nextCursor !== undefined;
+
+  if (!hadResumablePosition) {
+    return { ...response, items: trimmedItems };
+  }
+
+  return {
+    ...response,
+    items: trimmedItems,
+    page: {
+      ...response.page,
+      // We positively know unconsumed rows exist: the ones we just
+      // trimmed off. `hasMore` must reflect that even though we can no
+      // longer say where to resume.
+      hasMore: true,
+      nextCursor: undefined,
+      truncated: true,
+    },
+  };
+};
+
 export const callToolWithOptionalPagination = async (
   client: ToolCaller,
   toolName: string,
@@ -82,22 +145,40 @@ export const callToolWithOptionalPagination = async (
 ): Promise<unknown> => {
   validateAutoPagination(config);
 
-  const initial = await client.callTool(toolName, args);
   const shouldAutoPaginate =
     config.pagesLimit !== undefined || config.maxItems !== undefined;
+  const pagesLimit = config.pagesLimit ?? 1;
+  const maxItems = config.maxItems;
+
+  const baseArgs = normalizeArgs(args);
+  const callerLimit = getCallerLimit(baseArgs);
+
+  // Never over-fetch: request exactly the rows still needed so the page
+  // boundary coincides with the maxItems boundary, and the producer's own
+  // nextCursor stays correct by construction. This applies to the initial
+  // call too - not just subsequent ones.
+  const initialRequestLimit =
+    shouldAutoPaginate && maxItems !== undefined
+      ? getRequestLimit(callerLimit, maxItems)
+      : undefined;
+  const initialArgs =
+    initialRequestLimit === undefined
+      ? args
+      : { ...baseArgs, limit: initialRequestLimit };
+
+  const initial = await client.callTool(toolName, initialArgs);
   if (!shouldAutoPaginate || !isPagedResponse(initial)) {
     return initial;
   }
 
-  const pagesLimit = config.pagesLimit ?? 1;
-  const maxItems = config.maxItems;
+  const initialAdjusted = applyRequestBudget(initial, initialRequestLimit);
 
   let pageCount = 1;
-  let cursor = initial.page.nextCursor;
+  let cursor = initialAdjusted.page.nextCursor;
   const merged: PagedResponse = {
-    ...initial,
-    items: getTrimmedItems(initial.items, maxItems),
-    page: { ...initial.page },
+    ...initialAdjusted,
+    items: getTrimmedItems(initialAdjusted.items, maxItems),
+    page: { ...initialAdjusted.page },
   };
 
   while (
@@ -106,20 +187,30 @@ export const callToolWithOptionalPagination = async (
     pageCount < pagesLimit &&
     (maxItems === undefined || merged.items.length < maxItems)
   ) {
+    const remainingBudget =
+      maxItems === undefined
+        ? undefined
+        : Math.max(0, maxItems - merged.items.length);
+    const requestLimit =
+      remainingBudget === undefined
+        ? undefined
+        : getRequestLimit(callerLimit, remainingBudget);
+
     const nextArgs = {
-      ...normalizeArgs(args),
+      ...baseArgs,
       cursor,
+      ...(requestLimit === undefined ? {} : { limit: requestLimit }),
     };
 
-    const next = await client.callTool(toolName, nextArgs);
-    if (!isPagedResponse(next)) {
+    const rawNext = await client.callTool(toolName, nextArgs);
+    if (!isPagedResponse(rawNext)) {
       break;
     }
 
-    if (next.page.reset) {
-      merged.items = getTrimmedItems(next.items, maxItems);
-      merged.page = { ...next.page };
-      cursor = next.page.nextCursor;
+    if (rawNext.page.reset) {
+      merged.items = getTrimmedItems(rawNext.items, maxItems);
+      merged.page = { ...rawNext.page };
+      cursor = rawNext.page.nextCursor;
       pageCount += 1;
 
       if (maxItems !== undefined && merged.items.length >= maxItems) {
@@ -129,6 +220,8 @@ export const callToolWithOptionalPagination = async (
       continue;
     }
 
+    const next = applyRequestBudget(rawNext, requestLimit);
+
     const remaining =
       maxItems === undefined
         ? next.items.length
@@ -144,6 +237,8 @@ export const callToolWithOptionalPagination = async (
     }
   }
 
+  // The reported `limit` reflects the producer's page size for the very
+  // first raw response, never our internally-reduced per-request limit.
   merged.page.limit = initial.page.limit;
   return merged;
 };