diff --git a/.changeset/follow-opaque-cursors.md b/.changeset/follow-opaque-cursors.md new file mode 100644 index 0000000000..a67a6278a5 --- /dev/null +++ b/.changeset/follow-opaque-cursors.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': patch +--- + +Follow repeated opaque cursors during automatic list pagination until the configured `listMaxPages` limit or server termination, instead of silently truncating results based on cursor values. diff --git a/packages/client/src/client/client.ts b/packages/client/src/client/client.ts index 0b386a63e8..d6e1079e9e 100644 --- a/packages/client/src/client/client.ts +++ b/packages/client/src/client/client.ts @@ -1698,8 +1698,9 @@ export class Client extends Protocol { * methods' no-`cursor` auto-aggregate path. Page 1's result object is * mutated in place (its items array is extended; `nextCursor` is * cleared); page-1 metadata (`ttlMs`, `cacheScope`, `_meta`) is preserved. - * A `nextCursor` that repeats stops the walk (defence against a - * non-converging server, mcp.d's `drainList` guard); + * The walk is bounded by `listMaxPages`, which prevents a non-converging + * server from keeping the aggregate request open without interpreting + * opaque cursor values. * {@linkcode ClientOptions.listMaxPages} is a hard cap — hitting it * throws, so a partial aggregate is never cached. The * captured-generation guard skips the write when a `list_changed` landed @@ -1732,9 +1733,8 @@ export class Client extends Protocol { const generation = this._cache.captureGeneration(method); const acc = (await this.request({ method, ...(baseParams && { params: { ...baseParams } }) }, options)) as R; let cursor = acc.nextCursor; - const seen = new Set(); let pages = 1; - while (cursor !== undefined && !seen.has(cursor)) { + while (cursor !== undefined) { if (this._listMaxPages !== 0 && pages >= this._listMaxPages) { throw new SdkError( SdkErrorCode.ListPaginationExceeded, @@ -1742,7 +1742,6 @@ export class Client extends Protocol { { method, listMaxPages: this._listMaxPages } ); } - seen.add(cursor); const page = (await this.request({ method, params: { ...baseParams, cursor } }, options)) as R; append(acc, page); cursor = page.nextCursor; diff --git a/packages/client/test/client/responseCache.test.ts b/packages/client/test/client/responseCache.test.ts index cec28ac6d8..a00dc9d881 100644 --- a/packages/client/test/client/responseCache.test.ts +++ b/packages/client/test/client/responseCache.test.ts @@ -326,11 +326,13 @@ interface ScriptOptions { listHint?: { ttlMs?: number; cacheScope?: 'public' | 'private' }; readHint?: { ttlMs?: number; cacheScope?: 'public' | 'private' }; serverInfo?: { name: string; version: string }; + nextCursors?: (string | undefined)[]; } async function scriptedModernServer(pages: Tool[][], opts: ScriptOptions = {}): Promise { const [clientTx, serverTx] = InMemoryTransport.createLinkedPair(); let lists = 0; + let pageIndex = 0; const wireCounts = new Map(); const params: ({ cursor?: string; _meta?: unknown } | undefined)[] = []; serverTx.onmessage = m => { @@ -352,8 +354,15 @@ async function scriptedModernServer(pages: Tool[][], opts: ScriptOptions = {}): lists++; params.push(r.params as { cursor?: string; _meta?: unknown } | undefined); const cursor = (r.params as { cursor?: string } | undefined)?.cursor; - const idx = cursor === undefined ? 0 : Number(cursor); - const next = idx + 1 < pages.length ? String(idx + 1) : undefined; + // Cursor values are opaque: advance the scripted page by request + // order instead of interpreting the cursor as a page number. + const idx = cursor === undefined ? (pageIndex = 0) : ++pageIndex; + const next = + opts.nextCursors && lists - 1 < opts.nextCursors.length + ? opts.nextCursors[lists - 1] + : idx + 1 < pages.length + ? String(idx + 1) + : undefined; void serverTx.send({ jsonrpc: '2.0', id: r.id, @@ -436,6 +445,18 @@ describe('Client response-cache substrate', () => { expect((JSON.parse(entry!.value) as { tools: Tool[] }).tools.map(t => t.name)).toEqual(['a', 'b']); }); + it('listTools() follows repeated opaque cursors until the page limit or server termination', async () => { + const { clientTx, listParams } = await scriptedModernServer([[TOOL_A], [TOOL_B], [TOOL_A]], { + nextCursors: ['same', 'same', undefined] + }); + const client = modernClient(); + await client.connect(clientTx); + + const { tools } = await client.listTools(); + expect(tools.map(t => t.name)).toEqual(['a', 'b', 'a']); + expect(listParams().map(p => p?.cursor)).toEqual([undefined, 'same', 'same']); + }); + it('the auto-aggregate path threads caller params (e.g. _meta trace context) into every page request', async () => { const { clientTx, listParams } = await scriptedModernServer([[TOOL_A], [TOOL_B], [TOOL_A]]); const client = modernClient();