From 5f01677d126819e17c51e2144f13da7547ccfcab Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 21:48:26 +0000 Subject: [PATCH 1/5] fix(cli): Split paginate() limits evenly and isolate cursors by CLI version paginate() used 100-row pages for any --limit over 100, so --limit 201 asked for a full third page to return one row. It now uses the fewest requests and splits the limit evenly across them (201 -> 3 x 67). CLI cursors from another format version, or damaged ones, are ignored with a warning and the walk restarts from the first page instead of failing or reaching the API. Stored -c next history is namespaced by CURSOR_FORMAT, so CLI versions never resume from each other's cursors after an upgrade or downgrade. Co-Authored-By: BYK --- packages/cli/AGENTS.md | 8 ++ packages/cli/src/lib/api/infrastructure.ts | 78 ++++++++++++------- packages/cli/src/lib/constants.ts | 10 +++ packages/cli/src/lib/db/pagination.ts | 26 +++++-- .../cli/test/lib/api/infrastructure.test.ts | 58 +++++++++----- .../cli/test/lib/api/project-dsns.test.ts | 14 ++-- .../cli/test/lib/db/cache-corruption.test.ts | 15 +++- .../lib/db/pagination.model-based.test.ts | 3 +- packages/cli/test/lib/db/pagination.test.ts | 55 ++++++++++++- packages/cli/test/lib/org-list.test.ts | 12 +-- .../cli/test/lib/paginate.property.test.ts | 22 ++++++ 11 files changed, 233 insertions(+), 68 deletions(-) diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 7500a0b74..8e42ab6b8 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -476,6 +476,14 @@ pass these cursors back through `paginate()`; never send them to an API request directly. Test pagination changes against the paginator models in `test/lib/paginate.property.test.ts`. +A fresh walk splits `--limit` evenly over the fewest requests (201 → 3 × 67), +so the last request never fetches a full page for a few rows. CLI cursors this +version cannot read (another `cli~` format, or a damaged one) are ignored +with a warning and the walk starts from the first page. Stored `-c next` +history is namespaced by `CURSOR_FORMAT` in `src/lib/constants.ts`, so CLI +versions never resume from each other's cursors after an upgrade or +downgrade. Bump `CURSOR_FORMAT` whenever the cursor format changes. + Reference template: `trace/list.ts`, `span/list.ts`, `dashboard/revisions.ts` ### ID Validation diff --git a/packages/cli/src/lib/api/infrastructure.ts b/packages/cli/src/lib/api/infrastructure.ts index 3d13761f2..122eb2380 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -19,18 +19,14 @@ import * as Sentry from "@sentry/node-core/light"; import { type GenericSchema, safeParse } from "valibot"; import { extractRequiredScopes } from "../api-scope.js"; +import { CURSOR_FORMAT } from "../constants.js"; import { type CredentialContext, getActiveEnvVarName, isEnvTokenActive, } from "../db/auth.js"; import { getEnv } from "../env.js"; -import { - ApiError, - AuthError, - stringifyUnknown, - ValidationError, -} from "../errors.js"; +import { ApiError, AuthError, stringifyUnknown } from "../errors.js"; import { logger } from "../logger.js"; import { resolveOrgRegion } from "../region.js"; import { @@ -483,7 +479,10 @@ export async function autoPaginate( } /** Marks cursors produced by {@link paginate}; Sentry cursors never use it. */ -const PAGE_CURSOR_PREFIX = "cli1~"; +const PAGE_CURSOR_PREFIX = `${CURSOR_FORMAT}~`; + +/** Any CLI cursor format, including ones from older or newer CLI versions. */ +const CLI_CURSOR_RE = /^cli\d+~/; /** * Where a {@link paginate} walk resumes: the server cursor that fetches the @@ -502,32 +501,53 @@ function encodePageCursor({ cursor, perPage, skip }: PagePosition): string { /** * Decode a cursor from {@link paginate}, or treat anything else as a raw - * server cursor (older stored cursors, or one pasted from an API response). + * server cursor (one pasted from an API response or older CLI output). + * + * A CLI cursor this version cannot read (another format version, or a + * damaged one) is ignored with a warning and the walk starts from the first + * page, so a stale cursor never fails a command or reaches the API. */ function decodePageCursor( raw: string | undefined, ): Partial & { cursor: string | undefined } { - if (!raw?.startsWith(PAGE_CURSOR_PREFIX)) { + if (!(raw && CLI_CURSOR_RE.test(raw))) { return { cursor: raw }; } - const [perPage, skip, ...server] = raw - .slice(PAGE_CURSOR_PREFIX.length) - .split("~"); - const position = { - perPage: Number(perPage), - skip: Number(skip), - cursor: server.join("~") || undefined, - }; - if ( - !Number.isInteger(position.perPage) || - position.perPage < 1 || - position.perPage > API_MAX_PER_PAGE || - !Number.isInteger(position.skip) || - position.skip < 0 - ) { - throw new ValidationError(`Invalid pagination cursor '${raw}'.`, "cursor"); + if (raw.startsWith(PAGE_CURSOR_PREFIX)) { + const [perPage, skip, ...server] = raw + .slice(PAGE_CURSOR_PREFIX.length) + .split("~"); + const position = { + perPage: Number(perPage), + skip: Number(skip), + cursor: server.join("~") || undefined, + }; + if ( + Number.isInteger(position.perPage) && + position.perPage >= 1 && + position.perPage <= API_MAX_PER_PAGE && + Number.isInteger(position.skip) && + position.skip >= 0 + ) { + return position; + } + } + logger.warn( + "Ignoring a pagination cursor this CLI version cannot read; starting from the first page.", + ); + return { cursor: undefined }; +} + +/** + * Page size for a walk that starts fresh: the fewest requests that can hold + * `limit` rows, split as evenly as possible so the last request is not a + * full page for a few rows (201 rows → 3 pages of 67, not 100 + 100 + 100). + */ +function pageSizeForLimit(limit: number): number { + if (limit <= API_MAX_PER_PAGE) { + return Math.max(1, limit); } - return position; + return Math.ceil(limit / Math.ceil(limit / API_MAX_PER_PAGE)); } /** @@ -538,7 +558,8 @@ function decodePageCursor( * owns the decisions those loops kept getting wrong (#1458, #1608): * * - Every request of a walk, including later `-c next` calls with a different - * `--limit`, uses the `per_page` of its first request. Sentry's page-number + * `--limit`, uses the `per_page` of its first request + * ({@link pageSizeForLimit}). Sentry's page-number * paginators (`OffsetPaginator`, used by projects, teams, members, * repositories, and dashboard revisions) put the page size in the cursor * (`::`), and pairing it with another `per_page` @@ -570,8 +591,7 @@ export async function paginate( ): Promise> { const limit = options.limit ?? defaultLimit; const start = decodePageCursor(options.cursor); - const perPage = - start.perPage ?? Math.max(1, Math.min(limit, API_MAX_PER_PAGE)); + const perPage = start.perPage ?? pageSizeForLimit(limit); let cursor = start.cursor; let skip = start.skip ?? 0; const rows: T[] = []; diff --git a/packages/cli/src/lib/constants.ts b/packages/cli/src/lib/constants.ts index 64d69790f..0d18215c0 100644 --- a/packages/cli/src/lib/constants.ts +++ b/packages/cli/src/lib/constants.ts @@ -25,6 +25,16 @@ export const DEFAULT_SENTRY_HOST = "sentry.io"; /** Default Sentry SaaS URL (control silo for OAuth and region discovery) */ export const DEFAULT_SENTRY_URL = `https://${DEFAULT_SENTRY_HOST}`; +/** + * Version tag of the CLI's pagination cursor format. + * + * It prefixes cursors returned by `paginate()` and namespaces the stored + * `-c next` / `-c prev` history, so CLI versions with a different format never + * read each other's cursors after an upgrade or downgrade. Bump it whenever + * the cursor format changes. + */ +export const CURSOR_FORMAT = "cli1"; + /** * Name of the JavaScript package directory — used as both a skip target * when walking project trees (DSN scanner, sourcemap discovery, init diff --git a/packages/cli/src/lib/db/pagination.ts b/packages/cli/src/lib/db/pagination.ts index b56811e5d..b2516c90d 100644 --- a/packages/cli/src/lib/db/pagination.ts +++ b/packages/cli/src/lib/db/pagination.ts @@ -12,6 +12,7 @@ * Cursors expire after a short TTL to prevent stale pagination. */ +import { CURSOR_FORMAT } from "../constants.js"; import { ValidationError } from "../errors.js"; import { CURSOR_KEYWORDS } from "../list-command.js"; import type { ResolvedTarget } from "../resolve-target.js"; @@ -23,6 +24,20 @@ import { runUpsert } from "./utils.js"; /** Default TTL for stored cursors: 5 minutes */ const CURSOR_TTL_MS = 5 * 60 * 1000; +/** + * Key under which a command's cursor history is stored. + * + * Namespaced by {@link CURSOR_FORMAT} so a CLI upgrade or downgrade never + * resumes from a cursor written in another format: each version only sees + * its own history, and `-c next` after switching versions reports that no + * next page is saved until the command runs once without `--cursor`. + * + * @param commandKey - Command identifier (e.g., "trace-list") + */ +export function paginationStorageKey(commandKey: string): string { + return `${commandKey}@${CURSOR_FORMAT}`; +} + /** Direction derived from a `--cursor` flag value. */ export type CursorDirection = "next" | "prev" | "first"; @@ -69,11 +84,12 @@ export function getPaginationState( contextKey: string, ): PaginationState | undefined { const db = getDatabase(); + const storageKey = paginationStorageKey(commandKey); const row = db .query( "SELECT cursor_stack, page_index, expires_at FROM pagination_cursors WHERE command_key = ? AND context = ?", ) - .get(commandKey, contextKey) as PaginationCursorRow | undefined; + .get(storageKey, contextKey) as PaginationCursorRow | undefined; if (!row) { return; @@ -82,7 +98,7 @@ export function getPaginationState( if (row.expires_at <= Date.now()) { db.query( "DELETE FROM pagination_cursors WHERE command_key = ? AND context = ?", - ).run(commandKey, contextKey); + ).run(storageKey, contextKey); return; } @@ -96,7 +112,7 @@ export function getPaginationState( if (!stack) { db.query( "DELETE FROM pagination_cursors WHERE command_key = ? AND context = ?", - ).run(commandKey, contextKey); + ).run(storageKey, contextKey); return; } @@ -122,7 +138,7 @@ function savePaginationState( db, "pagination_cursors", { - command_key: commandKey, + command_key: paginationStorageKey(commandKey), context: contextKey, cursor_stack: JSON.stringify(state.stack), page_index: state.index, @@ -311,7 +327,7 @@ export function clearPaginationState( const db = getDatabase(); db.query( "DELETE FROM pagination_cursors WHERE command_key = ? AND context = ?", - ).run(commandKey, contextKey); + ).run(paginationStorageKey(commandKey), contextKey); } /** diff --git a/packages/cli/test/lib/api/infrastructure.test.ts b/packages/cli/test/lib/api/infrastructure.test.ts index 6f84d1a3d..066be3dfe 100644 --- a/packages/cli/test/lib/api/infrastructure.test.ts +++ b/packages/cli/test/lib/api/infrastructure.test.ts @@ -13,11 +13,7 @@ import { unwrapResult, } from "../../../src/lib/api/infrastructure.js"; import { setAuthToken } from "../../../src/lib/db/auth.js"; -import { - ApiError, - HostScopeError, - ValidationError, -} from "../../../src/lib/errors.js"; +import { ApiError, HostScopeError } from "../../../src/lib/errors.js"; import { mockFetch, useEnvSandbox, useTestConfigDir } from "../../helpers.js"; describe("SDK error compatibility", () => { @@ -878,6 +874,7 @@ describe("paginate", () => { expect(spy).toHaveBeenCalledWith(25, undefined); }); + // Raw server cursors, e.g. from an older CLI or an API response. test("forwards initial cursor", async () => { const spy = vi.fn().mockResolvedValue({ data: [] }); await paginate({ cursor: "abc", limit: 5 }, spy); @@ -892,6 +889,22 @@ describe("paginate", () => { expect(spy).toHaveBeenCalledWith(5, undefined); }); + test.each([ + { limit: 101, perPage: 51 }, + { limit: 150, perPage: 75 }, + { limit: 200, perPage: 100 }, + { limit: 201, perPage: 67 }, + { limit: 250, perPage: 84 }, + { limit: 1000, perPage: 100 }, + ])( + "splits --limit $limit evenly into pages of $perPage", + async ({ limit, perPage }) => { + const spy = vi.fn().mockResolvedValue({ data: [] }); + await paginate({ limit }, spy); + expect(spy).toHaveBeenCalledWith(perPage, undefined); + }, + ); + // Cursor continuity across servers and limits is covered by // test/lib/paginate.property.test.ts; this pins the request sequence. test("keeps the walk's page size and resumes mid-page across -c next", async () => { @@ -901,17 +914,19 @@ describe("paginate", () => { return Promise.resolve({ data, nextCursor: String(offset + perPage) }); }); - const first = await paginate({ limit: 150 }, spy); - expect(first.data).toEqual(Array.from({ length: 150 }, (_, i) => i)); + const first = await paginate({ limit: 250 }, spy); + expect(first.data).toEqual(Array.from({ length: 250 }, (_, i) => i)); const second = await paginate({ limit: 10, cursor: first.nextCursor }, spy); - expect(second.data).toEqual(Array.from({ length: 10 }, (_, i) => 150 + i)); + expect(second.data).toEqual(Array.from({ length: 10 }, (_, i) => 250 + i)); - // The first call stops inside page 2, so the second refetches it with the + // The first call stops inside page 3, so the second refetches it with the // walk's page size rather than one derived from its own --limit. expect(spy.mock.calls).toEqual([ - [100, undefined], - [100, "100"], - [100, "100"], + [84, undefined], + [84, "84"], + [84, "168"], + [84, "168"], + [84, "252"], ]); }); @@ -920,14 +935,21 @@ describe("paginate", () => { expect(await paginate({ limit: 5 }, spy)).toEqual({ data: [1, 2] }); }); - test.each(["cli1~0~0~abc", "cli1~101~0~abc", "cli1~10~-1~abc", "cli1~x~0~"])( - "rejects the malformed CLI cursor %s", + // Cursors from another CLI version (after an upgrade or downgrade) or a + // damaged one must never fail the command or reach the API. + test.each([ + "cli2~100~0~abc", + "cli0~abc", + "cli1~0~0~abc", + "cli1~101~0~abc", + "cli1~10~-1~abc", + "cli1~x~0~", + ])( + "restarts from the first page on the unreadable cursor %s", async (cursor) => { const spy = vi.fn().mockResolvedValue({ data: [] }); - await expect(paginate({ limit: 5, cursor }, spy)).rejects.toThrow( - ValidationError, - ); - expect(spy).not.toHaveBeenCalled(); + await paginate({ limit: 5, cursor }, spy); + expect(spy.mock.calls).toEqual([[5, undefined]]); }, ); }); diff --git a/packages/cli/test/lib/api/project-dsns.test.ts b/packages/cli/test/lib/api/project-dsns.test.ts index 6ec9e8e37..afc6e3eda 100644 --- a/packages/cli/test/lib/api/project-dsns.test.ts +++ b/packages/cli/test/lib/api/project-dsns.test.ts @@ -160,10 +160,11 @@ describe.each([ ); }); - const first = await list({ limit: 150, cursor: "0:25:0" }); + // 149 rows split as 2 x 75, so the first call stops inside page 2. + const first = await list({ limit: 149, cursor: "0:25:0" }); expect(projectIds).toEqual(organization ? ["42"] : []); - expect(first.data).toHaveLength(150); - expect(first.nextCursor).toContain("0:125:0"); + expect(first.data).toHaveLength(149); + expect(first.nextCursor).toContain("0:100:0"); const second = await list({ limit: 50, cursor: first.nextCursor }); expect( @@ -172,10 +173,11 @@ describe.each([ return [query.get("per_page"), query.get("cursor")]; }), ).toEqual([ - ["100", "0:25:0"], - ["100", "0:125:0"], + ["75", "0:25:0"], + ["75", "0:100:0"], // -c next refetches the partly returned page with the walk's page size. - ["100", "0:125:0"], + ["75", "0:100:0"], + ["75", "0:175:0"], ]); expect(second.nextCursor).toBeUndefined(); expect([...first.data, ...second.data].map((key) => key.name)).toEqual( diff --git a/packages/cli/test/lib/db/cache-corruption.test.ts b/packages/cli/test/lib/db/cache-corruption.test.ts index f712842f3..00f138e0d 100644 --- a/packages/cli/test/lib/db/cache-corruption.test.ts +++ b/packages/cli/test/lib/db/cache-corruption.test.ts @@ -9,7 +9,10 @@ import { describe, expect, test } from "vitest"; import { getDatabase } from "../../../src/lib/db/index.js"; -import { getPaginationState } from "../../../src/lib/db/pagination.js"; +import { + getPaginationState, + paginationStorageKey, +} from "../../../src/lib/db/pagination.js"; import { useTestConfigDir } from "../../helpers.js"; useTestConfigDir("cache-corruption-"); @@ -24,7 +27,13 @@ function insertPaginationRow(cursorStack: string): void { `INSERT OR REPLACE INTO pagination_cursors (command_key, context, cursor_stack, page_index, expires_at) VALUES (?, ?, ?, ?, ?)`, - ).run(CMD_KEY, CTX_KEY, cursorStack, 0, Date.now() + 60_000); + ).run( + paginationStorageKey(CMD_KEY), + CTX_KEY, + cursorStack, + 0, + Date.now() + 60_000, + ); } function paginationRowCount(): number { @@ -33,7 +42,7 @@ function paginationRowCount(): number { .query( "SELECT COUNT(*) AS n FROM pagination_cursors WHERE command_key = ? AND context = ?", ) - .get(CMD_KEY, CTX_KEY) as { n: number }; + .get(paginationStorageKey(CMD_KEY), CTX_KEY) as { n: number }; return row.n; } diff --git a/packages/cli/test/lib/db/pagination.model-based.test.ts b/packages/cli/test/lib/db/pagination.model-based.test.ts index bd906c9de..d57294a80 100644 --- a/packages/cli/test/lib/db/pagination.model-based.test.ts +++ b/packages/cli/test/lib/db/pagination.model-based.test.ts @@ -33,6 +33,7 @@ import { clearPaginationState, getPaginationState, hasPreviousPage, + paginationStorageKey, resolveCursor, } from "../../../src/lib/db/pagination.js"; import { @@ -552,7 +553,7 @@ describe("model-based: pagination cursor stack", () => { const db = getDatabase(); db.query( "UPDATE pagination_cursors SET expires_at = ? WHERE command_key = ? AND context = ?", - ).run(Date.now() - 1000, CMD_KEY, CTX_KEY); + ).run(Date.now() - 1000, paginationStorageKey(CMD_KEY), CTX_KEY); // Should return undefined expect(getPaginationState(CMD_KEY, CTX_KEY)).toBeUndefined(); diff --git a/packages/cli/test/lib/db/pagination.test.ts b/packages/cli/test/lib/db/pagination.test.ts index 22274009c..03028ebb4 100644 --- a/packages/cli/test/lib/db/pagination.test.ts +++ b/packages/cli/test/lib/db/pagination.test.ts @@ -1,13 +1,18 @@ /** - * Unit tests for pagination context key builders. + * Unit tests for pagination context key builders and cursor storage. */ import { describe, expect, test } from "vitest"; +import { getDatabase } from "../../../src/lib/db/index.js"; import { + advancePaginationState, buildMultiTargetContextKey, buildOrgContextKey, buildPaginationContextKey, + clearPaginationState, + resolveCursor, } from "../../../src/lib/db/pagination.js"; +import { useTestConfigDir } from "../../helpers.js"; describe("buildPaginationContextKey", () => { test("builds simple org-scoped key", () => { @@ -77,3 +82,51 @@ describe("buildOrgContextKey", () => { expect(orgKey).toBe(directKey); }); }); + +describe("cursor storage across CLI versions", () => { + useTestConfigDir("pagination-versions-"); + + const CMD = "trace-list"; + const CTX = "host:sentry.io|type:org:my-org"; + + /** Write a row the way a CLI version without the format namespace did. */ + function writeUnversionedRow(stack: string[]): void { + getDatabase() + .query( + `INSERT OR REPLACE INTO pagination_cursors + (command_key, context, cursor_stack, page_index, expires_at) + VALUES (?, ?, ?, ?, ?)`, + ) + .run(CMD, CTX, JSON.stringify(stack), 0, Date.now() + 60_000); + } + + function unversionedStack(): string | undefined { + const row = getDatabase() + .query( + "SELECT cursor_stack FROM pagination_cursors WHERE command_key = ? AND context = ?", + ) + .get(CMD, CTX) as { cursor_stack: string } | undefined; + return row?.cursor_stack; + } + + test("ignores history written in another cursor format", () => { + writeUnversionedRow(["", "1735689600:0:0"]); + expect(() => resolveCursor("next", CMD, CTX)).toThrow( + "No next page saved for this query", + ); + }); + + test("never overwrites or clears another format's history", () => { + writeUnversionedRow(["", "1735689600:0:0"]); + advancePaginationState(CMD, CTX, "first", "cli1~100~0~abc"); + + expect(resolveCursor("next", CMD, CTX).cursor).toBe("cli1~100~0~abc"); + expect(unversionedStack()).toBe(JSON.stringify(["", "1735689600:0:0"])); + + clearPaginationState(CMD, CTX); + expect(() => resolveCursor("next", CMD, CTX)).toThrow( + "No next page saved for this query", + ); + expect(unversionedStack()).toBe(JSON.stringify(["", "1735689600:0:0"])); + }); +}); diff --git a/packages/cli/test/lib/org-list.test.ts b/packages/cli/test/lib/org-list.test.ts index d74029db1..83845c875 100644 --- a/packages/cli/test/lib/org-list.test.ts +++ b/packages/cli/test/lib/org-list.test.ts @@ -512,17 +512,18 @@ describe("handleOrgAll", () => { }); expect(result.items).toHaveLength(250); - // Full pages keep requests to a minimum; the cursor carries the page size - // because team and repo lists use page-number cursors. + // The fewest requests, split evenly instead of overfetching on the last + // one; the cursor carries the page size because team and repo lists use + // page-number cursors. expect(listPaginated.mock.calls.map((call) => call[1].perPage)).toEqual([ - 100, 100, 100, + 84, 84, 84, ]); // The mock always returns a nextCursor, so hasMore is true and // the cursor points to the first item beyond the fetched 250. expect(result.hasMore).toBe(true); // `-c next` with another --limit keeps the walk's page size and refetches - // the third page, skipping the 50 rows already returned. + // the third page, skipping the 82 rows already returned. listPaginated.mockClear(); const next = await handleOrgAll({ config, @@ -536,7 +537,8 @@ describe("handleOrgAll", () => { Array.from({ length: 30 }, (_, i) => String(250 + i)), ); expect(listPaginated.mock.calls.map((call) => call[1])).toEqual([ - { cursor: "200", perPage: 100 }, + { cursor: "168", perPage: 84 }, + { cursor: "252", perPage: 84 }, ]); }); }); diff --git a/packages/cli/test/lib/paginate.property.test.ts b/packages/cli/test/lib/paginate.property.test.ts index d48253732..3cd73e4d1 100644 --- a/packages/cli/test/lib/paginate.property.test.ts +++ b/packages/cli/test/lib/paginate.property.test.ts @@ -138,3 +138,25 @@ describe.each(paginators)("property: paginate over $name", ({ make }) => { ); }); }); + +test("property: a fresh walk uses the fewest requests without overfetching", async () => { + await fcAssert( + asyncProperty( + integer({ min: 1, max: API_MAX_PER_PAGE * 10 }), + async (limit) => { + const requested: number[] = []; + const fetchPage = genericOffsetPaginator(limit * 2); + await paginate({ limit }, (perPage, cursor) => { + requested.push(perPage); + return fetchPage(perPage, cursor); + }); + // e.g. 201 rows: 3 requests of 67, not 100 + 100 + 100. + const requests = Math.ceil(limit / API_MAX_PER_PAGE); + expect(requested).toHaveLength(requests); + const fetched = requested.reduce((sum, perPage) => sum + perPage, 0); + expect(fetched - limit).toBeLessThan(requests); + }, + ), + { numRuns: DEFAULT_NUM_RUNS }, + ); +}); From e6f5ee4d2cf721f0f77ffc7406c9842bdec97397 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 22:09:10 +0000 Subject: [PATCH 2/5] fix(cli): Resume raw page-number cursors with their own page size A raw OffsetPaginator cursor (`::`) passed to paginate() was paired with the even-split page size for the limit, so the server read the wrong window and skipped or repeated rows. Recover the page size from the cursor instead; other paginators put 0 or a sort value there and are unaffected. --- packages/cli/src/lib/api/infrastructure.ts | 28 +++++++++++++-- .../cli/test/lib/api/infrastructure.test.ts | 16 +++++++++ .../cli/test/lib/paginate.property.test.ts | 34 +++++++++++++++++-- 3 files changed, 73 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/lib/api/infrastructure.ts b/packages/cli/src/lib/api/infrastructure.ts index 122eb2380..9e23d723e 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -484,6 +484,9 @@ const PAGE_CURSOR_PREFIX = `${CURSOR_FORMAT}~`; /** Any CLI cursor format, including ones from older or newer CLI versions. */ const CLI_CURSOR_RE = /^cli\d+~/; +/** Sentry cursor `::`; `OffsetPaginator` puts its page size in `value`. */ +const SERVER_CURSOR_RE = /^(\d+):\d+:[01]$/; + /** * Where a {@link paginate} walk resumes: the server cursor that fetches the * page, the page size it must be fetched with, and how many of that page's @@ -510,8 +513,12 @@ function encodePageCursor({ cursor, perPage, skip }: PagePosition): string { function decodePageCursor( raw: string | undefined, ): Partial & { cursor: string | undefined } { - if (!(raw && CLI_CURSOR_RE.test(raw))) { - return { cursor: raw }; + if (!raw) { + return { cursor: undefined }; + } + if (!CLI_CURSOR_RE.test(raw)) { + const perPage = serverCursorPageSize(raw); + return perPage === undefined ? { cursor: raw } : { cursor: raw, perPage }; } if (raw.startsWith(PAGE_CURSOR_PREFIX)) { const [perPage, skip, ...server] = raw @@ -538,6 +545,21 @@ function decodePageCursor( return { cursor: undefined }; } +/** + * Page size a raw server cursor must be resumed with, if it carries one. + * + * `OffsetPaginator` cursors (`::`) are only valid + * with the page size that produced them. Other paginators put an offset of 0 + * or a sort value there; a sort value that happens to be in range only + * changes the page size, which they tolerate. + */ +function serverCursorPageSize(raw: string): number | undefined { + const value = Number(SERVER_CURSOR_RE.exec(raw)?.[1]); + return Number.isInteger(value) && value >= 1 && value <= API_MAX_PER_PAGE + ? value + : undefined; +} + /** * Page size for a walk that starts fresh: the fewest requests that can hold * `limit` rows, split as evenly as possible so the last request is not a @@ -571,7 +593,7 @@ function pageSizeForLimit(limit: number): number { * * Returned cursors are opaque CLI cursors; pass them back through this * function, never directly to an API request. Raw server cursors are accepted - * as a starting point. + * as a starting point, with the page size of a page-number cursor. * * Callers own region resolution and query building inside `fetchPage`, and * must send `perPage` as the request's `per_page` unchanged. diff --git a/packages/cli/test/lib/api/infrastructure.test.ts b/packages/cli/test/lib/api/infrastructure.test.ts index 066be3dfe..ea0abae2b 100644 --- a/packages/cli/test/lib/api/infrastructure.test.ts +++ b/packages/cli/test/lib/api/infrastructure.test.ts @@ -935,6 +935,22 @@ describe("paginate", () => { expect(await paginate({ limit: 5 }, spy)).toEqual({ data: [1, 2] }); }); + // A pasted page-number cursor is only valid with the page size it encodes. + test.each([ + { cursor: "100:2:0", perPage: 100 }, + { cursor: "25:4:0", perPage: 25 }, + { cursor: "0:200:0", perPage: 67 }, + { cursor: "1735689600000:0:0", perPage: 67 }, + { cursor: "abc", perPage: 67 }, + ])( + "resumes the raw cursor $cursor with pages of $perPage", + async ({ cursor, perPage }) => { + const spy = vi.fn().mockResolvedValue({ data: [] }); + await paginate({ limit: 201, cursor }, spy); + expect(spy.mock.calls).toEqual([[perPage, cursor]]); + }, + ); + // Cursors from another CLI version (after an upgrade or downgrade) or a // damaged one must never fail the command or reach the API. test.each([ diff --git a/packages/cli/test/lib/paginate.property.test.ts b/packages/cli/test/lib/paginate.property.test.ts index 3cd73e4d1..72d5ea3de 100644 --- a/packages/cli/test/lib/paginate.property.test.ts +++ b/packages/cli/test/lib/paginate.property.test.ts @@ -87,9 +87,14 @@ const paginators = [ * Follow `-c next` until the cursor runs out, like repeated CLI calls, using * the next limit from `limits` (cycled) on each call. */ -async function walk(fetchPage: FetchPage, total: number, limits: number[]) { +async function walk( + fetchPage: FetchPage, + total: number, + limits: number[], + start?: string, +) { const seen: number[] = []; - let cursor: string | undefined; + let cursor = start; // Each call returns at least one row while rows remain. for (let call = 0; call <= total; call += 1) { const limit = limits[call % limits.length] ?? 1; @@ -139,6 +144,31 @@ describe.each(paginators)("property: paginate over $name", ({ make }) => { }); }); +test("property: a pasted OffsetPaginator cursor resumes without skipping", async () => { + await fcAssert( + asyncProperty( + integer({ min: 1, max: API_MAX_PER_PAGE }), + integer({ min: 1, max: 5 }), + nat(API_MAX_PER_PAGE * 3), + limitsArb, + async (pageSize, page, extra, limits) => { + // A cursor as the API returned it, from a walk with another page size. + const start = pageSize * page; + const seen = await walk( + offsetPaginator(start + extra), + extra, + limits, + `${pageSize}:${page}:0`, + ); + expect(seen).toEqual( + Array.from({ length: extra }, (_, i) => start + i), + ); + }, + ), + { numRuns: DEFAULT_NUM_RUNS }, + ); +}); + test("property: a fresh walk uses the fewest requests without overfetching", async () => { await fcAssert( asyncProperty( From f9333f025f427f70ece3c5a7fcd6ad4e38245077 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 22:22:13 +0000 Subject: [PATCH 3/5] fix(cli): Return Sentry-compatible cursors from paginate() Review feedback: make cursors work across CLI versions in both directions instead of isolating them. paginate() now returns Sentry cursors. A page-boundary cursor is the server's cursor unchanged. A mid-page cursor is the page's cursor with extra zeros in is_prev recording how many rows were already returned (`84:2:000` skips 2). Sentry parses is_prev with int() (Cursor.from_string), so older CLIs and the API accept these and see the skipped rows again rather than missing any. The page size comes back from page-number cursors themselves (OffsetPaginator encodes it), so it no longer needs to live in the cursor. That makes cursors stored by older CLIs resumable, so the CURSOR_FORMAT namespace for stored -c next history is removed and old and new versions share it again. cli1~ cursors from nightly builds are still read, and remain the fallback for cursors Sentry did not issue. Co-Authored-By: BYK --- packages/cli/AGENTS.md | 31 ++-- packages/cli/src/lib/api/infrastructure.ts | 144 +++++++++++------- packages/cli/src/lib/constants.ts | 10 -- packages/cli/src/lib/db/pagination.ts | 26 +--- .../cli/test/lib/api/infrastructure.test.ts | 129 ++++++++++++---- .../cli/test/lib/api/project-dsns.test.ts | 10 +- .../cli/test/lib/db/cache-corruption.test.ts | 15 +- .../lib/db/pagination.model-based.test.ts | 3 +- packages/cli/test/lib/db/pagination.test.ts | 55 +------ .../cli/test/lib/paginate.property.test.ts | 24 +++ 10 files changed, 243 insertions(+), 204 deletions(-) diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 8e42ab6b8..61a227413 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -466,24 +466,23 @@ const { data, nextCursor } = await paginate( Pass `perPage` to the request unchanged. Sentry's page-number paginators (`OffsetPaginator`, used by projects, teams, members, repositories, and -dashboard revisions) encode the page size in the cursor, so a cursor is only -valid with the `per_page` that produced it. `paginate()` therefore returns -opaque CLI cursors (`cli1~~~`) that record the -walk's page size and how many rows of the current page were already returned. -A later `-c next` reuses that page size even when `--limit` changes, and -resumes mid-page by refetching the page and skipping returned rows. Always -pass these cursors back through `paginate()`; never send them to an API -request directly. Test pagination changes against the paginator models in +dashboard revisions) encode the page size in the cursor +(`::`), so a cursor is only valid with the +`per_page` that produced it. `paginate()` reads that page size back from the +cursor, so a later `-c next` reuses it even when `--limit` changes. A fresh +walk splits `--limit` evenly over the fewest requests (201 → 3 × 67). + +The cursors `paginate()` returns are Sentry cursors, so every CLI version and +the API accept them, and stored `-c next` history works across upgrades and +downgrades. When a call stops inside a page, the cursor points at that page +and records how many of its rows were already returned as extra zeros in +`is_prev` (`84:2:000` skips 2 rows; Sentry parses `is_prev` with `int()`). +`paginate()` refetches the page and skips them; anything else that sends the +cursor (with the page size it encodes, if any) gets those rows again, never +fewer. Keep returned cursors valid Sentry +cursors. Test pagination changes against the paginator models in `test/lib/paginate.property.test.ts`. -A fresh walk splits `--limit` evenly over the fewest requests (201 → 3 × 67), -so the last request never fetches a full page for a few rows. CLI cursors this -version cannot read (another `cli~` format, or a damaged one) are ignored -with a warning and the walk starts from the first page. Stored `-c next` -history is namespaced by `CURSOR_FORMAT` in `src/lib/constants.ts`, so CLI -versions never resume from each other's cursors after an upgrade or -downgrade. Bump `CURSOR_FORMAT` whenever the cursor format changes. - Reference template: `trace/list.ts`, `span/list.ts`, `dashboard/revisions.ts` ### ID Validation diff --git a/packages/cli/src/lib/api/infrastructure.ts b/packages/cli/src/lib/api/infrastructure.ts index 9e23d723e..15e64cc55 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -19,7 +19,6 @@ import * as Sentry from "@sentry/node-core/light"; import { type GenericSchema, safeParse } from "valibot"; import { extractRequiredScopes } from "../api-scope.js"; -import { CURSOR_FORMAT } from "../constants.js"; import { type CredentialContext, getActiveEnvVarName, @@ -478,14 +477,29 @@ export async function autoPaginate( return { data: allRows.slice(0, limit) }; } -/** Marks cursors produced by {@link paginate}; Sentry cursors never use it. */ -const PAGE_CURSOR_PREFIX = `${CURSOR_FORMAT}~`; +/** + * A Sentry cursor, `::`, split before `is_prev`. + * + * Sentry parses `is_prev` with `int()` (`Cursor.from_string` in + * `src/sentry/utils/cursors.py`), so `0`, `00`, and `000` all mean "next". + * {@link paginate} uses the extra zeros to record how many rows of the page + * were already returned, which keeps its cursors valid Sentry cursors that + * any CLI version, or the API itself, accepts. + */ +const NEXT_CURSOR_RE = /^(.*:\d+):0(0*)$/; + +/** Resumes at the first row for every paginator the CLI uses. */ +const FIRST_PAGE_CURSOR = "0:0:0"; -/** Any CLI cursor format, including ones from older or newer CLI versions. */ -const CLI_CURSOR_RE = /^cli\d+~/; +/** `OffsetPaginator` cursors are `::`. */ +const PAGE_NUMBER_CURSOR_RE = /^(\d+):\d+:[01]$/; -/** Sentry cursor `::`; `OffsetPaginator` puts its page size in `value`. */ -const SERVER_CURSOR_RE = /^(\d+):\d+:[01]$/; +/** + * Prefix of cursors that carry the page size and skip outside the Sentry + * cursor. Only used when the server cursor is not a Sentry cursor, and read + * for cursors saved by nightly builds that always used it. + */ +const CLI_CURSOR_PREFIX = "cli1~"; /** * Where a {@link paginate} walk resumes: the server cursor that fetches the @@ -499,16 +513,22 @@ type PagePosition = { }; function encodePageCursor({ cursor, perPage, skip }: PagePosition): string { - return `${PAGE_CURSOR_PREFIX}${perPage}~${skip}~${cursor ?? ""}`; + const server = cursor ?? FIRST_PAGE_CURSOR; + if (skip === 0) { + return server; + } + const next = NEXT_CURSOR_RE.exec(server); + if (next) { + return `${next[1]}:${"0".repeat(skip + 1)}`; + } + return `${CLI_CURSOR_PREFIX}${perPage}~${skip}~${server}`; } /** - * Decode a cursor from {@link paginate}, or treat anything else as a raw - * server cursor (one pasted from an API response or older CLI output). + * Decode a cursor from {@link paginate}, an older CLI, or an API response. * - * A CLI cursor this version cannot read (another format version, or a - * damaged one) is ignored with a warning and the walk starts from the first - * page, so a stale cursor never fails a command or reaches the API. + * A damaged `cli1~` cursor is ignored with a warning and the walk starts from + * the first page, so it never fails a command or reaches the API. */ function decodePageCursor( raw: string | undefined, @@ -516,45 +536,55 @@ function decodePageCursor( if (!raw) { return { cursor: undefined }; } - if (!CLI_CURSOR_RE.test(raw)) { - const perPage = serverCursorPageSize(raw); - return perPage === undefined ? { cursor: raw } : { cursor: raw, perPage }; + if (raw.startsWith(CLI_CURSOR_PREFIX)) { + return decodeCliCursor(raw); } - if (raw.startsWith(PAGE_CURSOR_PREFIX)) { - const [perPage, skip, ...server] = raw - .slice(PAGE_CURSOR_PREFIX.length) - .split("~"); - const position = { - perPage: Number(perPage), - skip: Number(skip), - cursor: server.join("~") || undefined, - }; - if ( - Number.isInteger(position.perPage) && - position.perPage >= 1 && - position.perPage <= API_MAX_PER_PAGE && - Number.isInteger(position.skip) && - position.skip >= 0 - ) { - return position; - } + const next = NEXT_CURSOR_RE.exec(raw); + const cursor = next ? `${next[1]}:0` : raw; + const position = { + cursor: cursor === FIRST_PAGE_CURSOR ? undefined : cursor, + skip: next?.[2]?.length ?? 0, + }; + const perPage = serverCursorPageSize(cursor); + return perPage === undefined ? position : { ...position, perPage }; +} + +function decodeCliCursor( + raw: string, +): Partial & { cursor: string | undefined } { + const [perPage, skip, ...server] = raw + .slice(CLI_CURSOR_PREFIX.length) + .split("~"); + const position = { + perPage: Number(perPage), + skip: Number(skip), + cursor: server.join("~") || undefined, + }; + if ( + Number.isInteger(position.perPage) && + position.perPage >= 1 && + position.perPage <= API_MAX_PER_PAGE && + Number.isInteger(position.skip) && + position.skip >= 0 + ) { + return position; } logger.warn( - "Ignoring a pagination cursor this CLI version cannot read; starting from the first page.", + "Ignoring an unreadable pagination cursor; starting from the first page.", ); return { cursor: undefined }; } /** - * Page size a raw server cursor must be resumed with, if it carries one. + * Page size a server cursor must be resumed with, if it carries one. * - * `OffsetPaginator` cursors (`::`) are only valid - * with the page size that produced them. Other paginators put an offset of 0 - * or a sort value there; a sort value that happens to be in range only - * changes the page size, which they tolerate. + * `OffsetPaginator` cursors are only valid with the page size that produced + * them. Other paginators put an offset of 0 or a sort value there; a sort + * value that happens to be in range only changes the page size, which they + * tolerate. */ -function serverCursorPageSize(raw: string): number | undefined { - const value = Number(SERVER_CURSOR_RE.exec(raw)?.[1]); +function serverCursorPageSize(cursor: string): number | undefined { + const value = Number(PAGE_NUMBER_CURSOR_RE.exec(cursor)?.[1]); return Number.isInteger(value) && value >= 1 && value <= API_MAX_PER_PAGE ? value : undefined; @@ -579,21 +609,23 @@ function pageSizeForLimit(limit: number): number { * `sentry-cli/no-manual-pagination` lint rule rejects hand-written loops. It * owns the decisions those loops kept getting wrong (#1458, #1608): * - * - Every request of a walk, including later `-c next` calls with a different - * `--limit`, uses the `per_page` of its first request - * ({@link pageSizeForLimit}). Sentry's page-number - * paginators (`OffsetPaginator`, used by projects, teams, members, - * repositories, and dashboard revisions) put the page size in the cursor - * (`::`), and pairing it with another `per_page` - * reads the wrong window and silently skips rows. + * - A fresh walk splits `limit` evenly over the fewest requests + * ({@link pageSizeForLimit}). Resuming a page-number cursor + * (`OffsetPaginator`, used by projects, teams, members, repositories, and + * dashboard revisions) reuses the page size the cursor encodes + * (`::`), whatever the new `limit`: pairing it with + * another `per_page` reads the wrong window and silently skips rows. * - The returned cursor always resumes at the first row the caller has not * received. When a call stops inside a page (a smaller `--limit`, or a * server that ignored `per_page`), the cursor refetches that page and skips * the rows already returned. * - * Returned cursors are opaque CLI cursors; pass them back through this - * function, never directly to an API request. Raw server cursors are accepted - * as a starting point, with the page size of a page-number cursor. + * Returned cursors are Sentry cursors, so older CLI versions and the API + * accept them too; mid-page cursors add zeros to `is_prev` to record the rows + * to skip ({@link NEXT_CURSOR_RE}), which a caller that does not know about + * them sees again rather than misses (given the page size a page-number + * cursor encodes). Pass cursors back through this function + * to resume exactly. Raw server cursors are accepted as a starting point. * * Callers own region resolution and query building inside `fetchPage`, and * must send `perPage` as the request's `per_page` unchanged. @@ -613,9 +645,10 @@ export async function paginate( ): Promise> { const limit = options.limit ?? defaultLimit; const start = decodePageCursor(options.cursor); - const perPage = start.perPage ?? pageSizeForLimit(limit); let cursor = start.cursor; let skip = start.skip ?? 0; + // Cursors without a page size accept any, so fetch the skipped rows too. + const perPage = start.perPage ?? pageSizeForLimit(limit + skip); const rows: T[] = []; for (let page = 0; page < MAX_PAGINATION_PAGES; page += 1) { @@ -639,7 +672,8 @@ export async function paginate( return { data: rows }; } cursor = nextCursor; - skip = 0; + // A resumed walk can fetch pages smaller than the rows it must skip. + skip = Math.max(0, skip - data.length); if (rows.length === limit) { break; } @@ -653,7 +687,7 @@ export async function paginate( } return { data: rows, - nextCursor: encodePageCursor({ cursor, perPage, skip: 0 }), + nextCursor: encodePageCursor({ cursor, perPage, skip }), }; } diff --git a/packages/cli/src/lib/constants.ts b/packages/cli/src/lib/constants.ts index 0d18215c0..64d69790f 100644 --- a/packages/cli/src/lib/constants.ts +++ b/packages/cli/src/lib/constants.ts @@ -25,16 +25,6 @@ export const DEFAULT_SENTRY_HOST = "sentry.io"; /** Default Sentry SaaS URL (control silo for OAuth and region discovery) */ export const DEFAULT_SENTRY_URL = `https://${DEFAULT_SENTRY_HOST}`; -/** - * Version tag of the CLI's pagination cursor format. - * - * It prefixes cursors returned by `paginate()` and namespaces the stored - * `-c next` / `-c prev` history, so CLI versions with a different format never - * read each other's cursors after an upgrade or downgrade. Bump it whenever - * the cursor format changes. - */ -export const CURSOR_FORMAT = "cli1"; - /** * Name of the JavaScript package directory — used as both a skip target * when walking project trees (DSN scanner, sourcemap discovery, init diff --git a/packages/cli/src/lib/db/pagination.ts b/packages/cli/src/lib/db/pagination.ts index b2516c90d..b56811e5d 100644 --- a/packages/cli/src/lib/db/pagination.ts +++ b/packages/cli/src/lib/db/pagination.ts @@ -12,7 +12,6 @@ * Cursors expire after a short TTL to prevent stale pagination. */ -import { CURSOR_FORMAT } from "../constants.js"; import { ValidationError } from "../errors.js"; import { CURSOR_KEYWORDS } from "../list-command.js"; import type { ResolvedTarget } from "../resolve-target.js"; @@ -24,20 +23,6 @@ import { runUpsert } from "./utils.js"; /** Default TTL for stored cursors: 5 minutes */ const CURSOR_TTL_MS = 5 * 60 * 1000; -/** - * Key under which a command's cursor history is stored. - * - * Namespaced by {@link CURSOR_FORMAT} so a CLI upgrade or downgrade never - * resumes from a cursor written in another format: each version only sees - * its own history, and `-c next` after switching versions reports that no - * next page is saved until the command runs once without `--cursor`. - * - * @param commandKey - Command identifier (e.g., "trace-list") - */ -export function paginationStorageKey(commandKey: string): string { - return `${commandKey}@${CURSOR_FORMAT}`; -} - /** Direction derived from a `--cursor` flag value. */ export type CursorDirection = "next" | "prev" | "first"; @@ -84,12 +69,11 @@ export function getPaginationState( contextKey: string, ): PaginationState | undefined { const db = getDatabase(); - const storageKey = paginationStorageKey(commandKey); const row = db .query( "SELECT cursor_stack, page_index, expires_at FROM pagination_cursors WHERE command_key = ? AND context = ?", ) - .get(storageKey, contextKey) as PaginationCursorRow | undefined; + .get(commandKey, contextKey) as PaginationCursorRow | undefined; if (!row) { return; @@ -98,7 +82,7 @@ export function getPaginationState( if (row.expires_at <= Date.now()) { db.query( "DELETE FROM pagination_cursors WHERE command_key = ? AND context = ?", - ).run(storageKey, contextKey); + ).run(commandKey, contextKey); return; } @@ -112,7 +96,7 @@ export function getPaginationState( if (!stack) { db.query( "DELETE FROM pagination_cursors WHERE command_key = ? AND context = ?", - ).run(storageKey, contextKey); + ).run(commandKey, contextKey); return; } @@ -138,7 +122,7 @@ function savePaginationState( db, "pagination_cursors", { - command_key: paginationStorageKey(commandKey), + command_key: commandKey, context: contextKey, cursor_stack: JSON.stringify(state.stack), page_index: state.index, @@ -327,7 +311,7 @@ export function clearPaginationState( const db = getDatabase(); db.query( "DELETE FROM pagination_cursors WHERE command_key = ? AND context = ?", - ).run(paginationStorageKey(commandKey), contextKey); + ).run(commandKey, contextKey); } /** diff --git a/packages/cli/test/lib/api/infrastructure.test.ts b/packages/cli/test/lib/api/infrastructure.test.ts index ea0abae2b..d39c4cd0e 100644 --- a/packages/cli/test/lib/api/infrastructure.test.ts +++ b/packages/cli/test/lib/api/infrastructure.test.ts @@ -906,28 +906,90 @@ describe("paginate", () => { ); // Cursor continuity across servers and limits is covered by - // test/lib/paginate.property.test.ts; this pins the request sequence. - test("keeps the walk's page size and resumes mid-page across -c next", async () => { - const spy = vi.fn((perPage: number, cursor: string | undefined) => { - const offset = cursor ? Number(cursor) : 0; - const data = Array.from({ length: perPage }, (_, i) => offset + i); - return Promise.resolve({ data, nextCursor: String(offset + perPage) }); + // test/lib/paginate.property.test.ts; these pin the request sequence. + describe("resumes mid-page across -c next", () => { + /** Pages `rows` like Sentry, with `::0` cursors. */ + function server(cursorFor: (perPage: number, offset: number) => string) { + return vi.fn((perPage: number, cursor: string | undefined) => { + const [value = 0, page = 0] = (cursor ?? "0:0:0") + .split(":") + .map(Number); + const offset = value ? value * page : page; + const data = Array.from({ length: perPage }, (_, i) => offset + i); + return Promise.resolve({ + data, + nextCursor: cursorFor(perPage, offset), + }); + }); + } + + test("with the page size of a page-number cursor", async () => { + const spy = server( + (perPage, offset) => `${perPage}:${offset / perPage + 1}:0`, + ); + + const first = await paginate({ limit: 250 }, spy); + expect(first.data).toEqual(Array.from({ length: 250 }, (_, i) => i)); + // Page 84:2 was partly returned: 82 rows to skip, as zeros in is_prev. + expect(first.nextCursor).toBe(`84:2:${"0".repeat(83)}`); + const second = await paginate( + { limit: 10, cursor: first.nextCursor }, + spy, + ); + expect(second.data).toEqual( + Array.from({ length: 10 }, (_, i) => 250 + i), + ); + + expect(spy.mock.calls).toEqual([ + [84, undefined], + [84, "84:1:0"], + [84, "84:2:0"], + [84, "84:2:0"], + [84, "84:3:0"], + ]); }); - const first = await paginate({ limit: 250 }, spy); - expect(first.data).toEqual(Array.from({ length: 250 }, (_, i) => i)); - const second = await paginate({ limit: 10, cursor: first.nextCursor }, spy); - expect(second.data).toEqual(Array.from({ length: 10 }, (_, i) => 250 + i)); - - // The first call stops inside page 3, so the second refetches it with the - // walk's page size rather than one derived from its own --limit. - expect(spy.mock.calls).toEqual([ - [84, undefined], - [84, "84"], - [84, "168"], - [84, "168"], - [84, "252"], - ]); + test("fetching the skipped rows in the same request otherwise", async () => { + const spy = server((perPage, offset) => `0:${offset + perPage}:0`); + + const first = await paginate({ limit: 250 }, spy); + expect(first.nextCursor).toBe(`0:168:${"0".repeat(83)}`); + const second = await paginate( + { limit: 10, cursor: first.nextCursor }, + spy, + ); + expect(second.data).toEqual( + Array.from({ length: 10 }, (_, i) => 250 + i), + ); + expect(second.nextCursor).toBe("0:260:0"); + + expect(spy.mock.calls.slice(3)).toEqual([[92, "0:168:0"]]); + }); + + test("from the first page", async () => { + const spy = server((perPage, offset) => `0:${offset + perPage}:0`); + + const first = await paginate({ limit: 3 }, (perPage, cursor) => + spy(perPage + 2, cursor), + ); + expect(first.nextCursor).toBe("0:0:0000"); + const second = await paginate( + { limit: 2, cursor: first.nextCursor }, + spy, + ); + expect(second.data).toEqual([3, 4]); + expect(spy.mock.calls.at(-1)).toEqual([5, undefined]); + }); + }); + + test("returns page-boundary cursors unchanged", async () => { + const spy = vi + .fn() + .mockResolvedValue({ data: [1, 2], nextCursor: "2:1:0" }); + expect(await paginate({ limit: 2 }, spy)).toEqual({ + data: [1, 2], + nextCursor: "2:1:0", + }); }); test("returns no cursor once the server is exhausted", async () => { @@ -951,16 +1013,23 @@ describe("paginate", () => { }, ); - // Cursors from another CLI version (after an upgrade or downgrade) or a - // damaged one must never fail the command or reach the API. - test.each([ - "cli2~100~0~abc", - "cli0~abc", - "cli1~0~0~abc", - "cli1~101~0~abc", - "cli1~10~-1~abc", - "cli1~x~0~", - ])( + test("keeps the page size and skip outside cursors Sentry did not issue", async () => { + const spy = vi.fn().mockResolvedValue({ + data: [1, 2, 3], + nextCursor: "opaque", + }); + const first = await paginate({ limit: 2, cursor: "token" }, spy); + expect(first.nextCursor).toBe("cli1~2~2~token"); + }); + + // Nightly builds returned cli1~~~ cursors. + test("resumes a nightly cli1~ cursor", async () => { + const spy = vi.fn().mockResolvedValue({ data: [] }); + await paginate({ limit: 5, cursor: "cli1~84~2~84:2:0" }, spy); + expect(spy.mock.calls).toEqual([[84, "84:2:0"]]); + }); + + test.each(["cli1~0~0~abc", "cli1~101~0~abc", "cli1~10~-1~abc", "cli1~x~0~"])( "restarts from the first page on the unreadable cursor %s", async (cursor) => { const spy = vi.fn().mockResolvedValue({ data: [] }); diff --git a/packages/cli/test/lib/api/project-dsns.test.ts b/packages/cli/test/lib/api/project-dsns.test.ts index afc6e3eda..4e0f2693a 100644 --- a/packages/cli/test/lib/api/project-dsns.test.ts +++ b/packages/cli/test/lib/api/project-dsns.test.ts @@ -164,7 +164,8 @@ describe.each([ const first = await list({ limit: 149, cursor: "0:25:0" }); expect(projectIds).toEqual(organization ? ["42"] : []); expect(first.data).toHaveLength(149); - expect(first.nextCursor).toContain("0:100:0"); + // Page 0:100 was partly returned: 74 rows to skip, as zeros in is_prev. + expect(first.nextCursor).toBe(`0:100:${"0".repeat(75)}`); const second = await list({ limit: 50, cursor: first.nextCursor }); expect( @@ -175,9 +176,10 @@ describe.each([ ).toEqual([ ["75", "0:25:0"], ["75", "0:100:0"], - // -c next refetches the partly returned page with the walk's page size. - ["75", "0:100:0"], - ["75", "0:175:0"], + // -c next refetches the partly returned page; offset cursors take any + // page size, so it splits the 74 skipped + 50 requested rows evenly. + ["62", "0:100:0"], + ["62", "0:162:0"], ]); expect(second.nextCursor).toBeUndefined(); expect([...first.data, ...second.data].map((key) => key.name)).toEqual( diff --git a/packages/cli/test/lib/db/cache-corruption.test.ts b/packages/cli/test/lib/db/cache-corruption.test.ts index 00f138e0d..f712842f3 100644 --- a/packages/cli/test/lib/db/cache-corruption.test.ts +++ b/packages/cli/test/lib/db/cache-corruption.test.ts @@ -9,10 +9,7 @@ import { describe, expect, test } from "vitest"; import { getDatabase } from "../../../src/lib/db/index.js"; -import { - getPaginationState, - paginationStorageKey, -} from "../../../src/lib/db/pagination.js"; +import { getPaginationState } from "../../../src/lib/db/pagination.js"; import { useTestConfigDir } from "../../helpers.js"; useTestConfigDir("cache-corruption-"); @@ -27,13 +24,7 @@ function insertPaginationRow(cursorStack: string): void { `INSERT OR REPLACE INTO pagination_cursors (command_key, context, cursor_stack, page_index, expires_at) VALUES (?, ?, ?, ?, ?)`, - ).run( - paginationStorageKey(CMD_KEY), - CTX_KEY, - cursorStack, - 0, - Date.now() + 60_000, - ); + ).run(CMD_KEY, CTX_KEY, cursorStack, 0, Date.now() + 60_000); } function paginationRowCount(): number { @@ -42,7 +33,7 @@ function paginationRowCount(): number { .query( "SELECT COUNT(*) AS n FROM pagination_cursors WHERE command_key = ? AND context = ?", ) - .get(paginationStorageKey(CMD_KEY), CTX_KEY) as { n: number }; + .get(CMD_KEY, CTX_KEY) as { n: number }; return row.n; } diff --git a/packages/cli/test/lib/db/pagination.model-based.test.ts b/packages/cli/test/lib/db/pagination.model-based.test.ts index d57294a80..bd906c9de 100644 --- a/packages/cli/test/lib/db/pagination.model-based.test.ts +++ b/packages/cli/test/lib/db/pagination.model-based.test.ts @@ -33,7 +33,6 @@ import { clearPaginationState, getPaginationState, hasPreviousPage, - paginationStorageKey, resolveCursor, } from "../../../src/lib/db/pagination.js"; import { @@ -553,7 +552,7 @@ describe("model-based: pagination cursor stack", () => { const db = getDatabase(); db.query( "UPDATE pagination_cursors SET expires_at = ? WHERE command_key = ? AND context = ?", - ).run(Date.now() - 1000, paginationStorageKey(CMD_KEY), CTX_KEY); + ).run(Date.now() - 1000, CMD_KEY, CTX_KEY); // Should return undefined expect(getPaginationState(CMD_KEY, CTX_KEY)).toBeUndefined(); diff --git a/packages/cli/test/lib/db/pagination.test.ts b/packages/cli/test/lib/db/pagination.test.ts index 03028ebb4..22274009c 100644 --- a/packages/cli/test/lib/db/pagination.test.ts +++ b/packages/cli/test/lib/db/pagination.test.ts @@ -1,18 +1,13 @@ /** - * Unit tests for pagination context key builders and cursor storage. + * Unit tests for pagination context key builders. */ import { describe, expect, test } from "vitest"; -import { getDatabase } from "../../../src/lib/db/index.js"; import { - advancePaginationState, buildMultiTargetContextKey, buildOrgContextKey, buildPaginationContextKey, - clearPaginationState, - resolveCursor, } from "../../../src/lib/db/pagination.js"; -import { useTestConfigDir } from "../../helpers.js"; describe("buildPaginationContextKey", () => { test("builds simple org-scoped key", () => { @@ -82,51 +77,3 @@ describe("buildOrgContextKey", () => { expect(orgKey).toBe(directKey); }); }); - -describe("cursor storage across CLI versions", () => { - useTestConfigDir("pagination-versions-"); - - const CMD = "trace-list"; - const CTX = "host:sentry.io|type:org:my-org"; - - /** Write a row the way a CLI version without the format namespace did. */ - function writeUnversionedRow(stack: string[]): void { - getDatabase() - .query( - `INSERT OR REPLACE INTO pagination_cursors - (command_key, context, cursor_stack, page_index, expires_at) - VALUES (?, ?, ?, ?, ?)`, - ) - .run(CMD, CTX, JSON.stringify(stack), 0, Date.now() + 60_000); - } - - function unversionedStack(): string | undefined { - const row = getDatabase() - .query( - "SELECT cursor_stack FROM pagination_cursors WHERE command_key = ? AND context = ?", - ) - .get(CMD, CTX) as { cursor_stack: string } | undefined; - return row?.cursor_stack; - } - - test("ignores history written in another cursor format", () => { - writeUnversionedRow(["", "1735689600:0:0"]); - expect(() => resolveCursor("next", CMD, CTX)).toThrow( - "No next page saved for this query", - ); - }); - - test("never overwrites or clears another format's history", () => { - writeUnversionedRow(["", "1735689600:0:0"]); - advancePaginationState(CMD, CTX, "first", "cli1~100~0~abc"); - - expect(resolveCursor("next", CMD, CTX).cursor).toBe("cli1~100~0~abc"); - expect(unversionedStack()).toBe(JSON.stringify(["", "1735689600:0:0"])); - - clearPaginationState(CMD, CTX); - expect(() => resolveCursor("next", CMD, CTX)).toThrow( - "No next page saved for this query", - ); - expect(unversionedStack()).toBe(JSON.stringify(["", "1735689600:0:0"])); - }); -}); diff --git a/packages/cli/test/lib/paginate.property.test.ts b/packages/cli/test/lib/paginate.property.test.ts index 72d5ea3de..4a34c7c89 100644 --- a/packages/cli/test/lib/paginate.property.test.ts +++ b/packages/cli/test/lib/paginate.property.test.ts @@ -133,6 +133,30 @@ describe.each(paginators)("property: paginate over $name", ({ make }) => { ); }); + // What an older CLI does with a cursor: send it to the API unchanged. Rows + // the cursor marks as returned come back again, but none are skipped. + test("returns cursors the API resumes from without skipping rows", async () => { + await fcAssert( + asyncProperty(totalArb, limitsArb, async (total, [limit = 1]) => { + const fetchPage = make(total); + let perPage = 0; + const result = await paginate({ limit }, (size, cursor) => { + perPage = size; + return fetchPage(size, cursor); + }); + if (!result.nextCursor) { + return; + } + expect(result.nextCursor).toMatch(/^\d+:\d+:0+$/); + const { data } = await fetchPage(perPage, result.nextCursor); + const next = result.data.length; + expect(data[0]).toBeLessThanOrEqual(next); + expect(data).toContain(next); + }), + { numRuns: DEFAULT_NUM_RUNS }, + ); + }); + test("returns every row exactly once, in order, across -c next", async () => { await fcAssert( asyncProperty(totalArb, limitsArb, async (total, limits) => { From 9c3fbb5470e21a6ec1f71fd721cb0aba3ecc5bf8 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 22:38:15 +0000 Subject: [PATCH 4/5] fix(cli): Never write cli1~ cursors The cli1~ fallback for server cursors that cannot carry a skip (opaque strings, is_prev=1) was the one path that still produced cursors older CLIs cannot send to the API, and stored -c next history is shared across versions again. Return the page's server cursor instead; resuming returns that page's rows again rather than missing any. cli1~ cursors from nightly builds are still read. --- packages/cli/src/lib/api/infrastructure.ts | 31 ++++++++----------- .../cli/test/lib/api/infrastructure.test.ts | 19 +++++++----- packages/cli/test/lib/org-list.test.ts | 12 ++++--- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/packages/cli/src/lib/api/infrastructure.ts b/packages/cli/src/lib/api/infrastructure.ts index 15e64cc55..4a49cbf6c 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -495,9 +495,9 @@ const FIRST_PAGE_CURSOR = "0:0:0"; const PAGE_NUMBER_CURSOR_RE = /^(\d+):\d+:[01]$/; /** - * Prefix of cursors that carry the page size and skip outside the Sentry - * cursor. Only used when the server cursor is not a Sentry cursor, and read - * for cursors saved by nightly builds that always used it. + * Prefix of `cli1~~~` cursors, which nightly + * builds returned. Read so their saved history still resumes; never written, + * because older CLIs send cursors to the API unchanged. */ const CLI_CURSOR_PREFIX = "cli1~"; @@ -512,16 +512,15 @@ type PagePosition = { skip: number; }; -function encodePageCursor({ cursor, perPage, skip }: PagePosition): string { +/** + * Encode where a walk resumes as a Sentry cursor. A server cursor that is not + * a Sentry next-page cursor cannot carry the skip, so it is returned as is and + * resuming returns that page's rows again rather than missing any. + */ +function encodePageCursor(cursor: string | undefined, skip: number): string { const server = cursor ?? FIRST_PAGE_CURSOR; - if (skip === 0) { - return server; - } - const next = NEXT_CURSOR_RE.exec(server); - if (next) { - return `${next[1]}:${"0".repeat(skip + 1)}`; - } - return `${CLI_CURSOR_PREFIX}${perPage}~${skip}~${server}`; + const next = skip > 0 ? NEXT_CURSOR_RE.exec(server) : null; + return next ? `${next[1]}:${"0".repeat(skip + 1)}` : server; } /** @@ -660,11 +659,7 @@ export async function paginate( rows.push(...unseen.slice(0, remaining)); return { data: rows, - nextCursor: encodePageCursor({ - cursor, - perPage, - skip: skip + remaining, - }), + nextCursor: encodePageCursor(cursor, skip + remaining), }; } rows.push(...unseen); @@ -687,7 +682,7 @@ export async function paginate( } return { data: rows, - nextCursor: encodePageCursor({ cursor, perPage, skip }), + nextCursor: encodePageCursor(cursor, skip), }; } diff --git a/packages/cli/test/lib/api/infrastructure.test.ts b/packages/cli/test/lib/api/infrastructure.test.ts index d39c4cd0e..90441997e 100644 --- a/packages/cli/test/lib/api/infrastructure.test.ts +++ b/packages/cli/test/lib/api/infrastructure.test.ts @@ -1013,14 +1013,17 @@ describe("paginate", () => { }, ); - test("keeps the page size and skip outside cursors Sentry did not issue", async () => { - const spy = vi.fn().mockResolvedValue({ - data: [1, 2, 3], - nextCursor: "opaque", - }); - const first = await paginate({ limit: 2, cursor: "token" }, spy); - expect(first.nextCursor).toBe("cli1~2~2~token"); - }); + // Every CLI version must be able to send a returned cursor to the API. + test.each(["token", "100:3:1"])( + "refetches the page of the cursor %s, which cannot carry a skip", + async (cursor) => { + const spy = vi + .fn() + .mockResolvedValue({ data: [1, 2, 3], nextCursor: "x" }); + const first = await paginate({ limit: 2, cursor }, spy); + expect(first.nextCursor).toBe(cursor); + }, + ); // Nightly builds returned cli1~~~ cursors. test("resumes a nightly cli1~ cursor", async () => { diff --git a/packages/cli/test/lib/org-list.test.ts b/packages/cli/test/lib/org-list.test.ts index 83845c875..163b430fa 100644 --- a/packages/cli/test/lib/org-list.test.ts +++ b/packages/cli/test/lib/org-list.test.ts @@ -489,14 +489,18 @@ describe("handleOrgAll", () => { const listPaginated = vi.fn( (_org: string, opts: { cursor?: string; perPage: number }) => { expect(opts.perPage).toBeLessThanOrEqual(100); - const offset = opts.cursor ? Number(opts.cursor) : 0; + // OffsetPaginator cursors: ::. + const [size = 0, page = 0] = (opts.cursor ?? "0:0:0") + .split(":") + .map(Number); + const offset = size * page; const data = Array.from({ length: opts.perPage }, (_, i) => ({ id: String(offset + i), name: `W${offset + i}`, })); return Promise.resolve({ data, - nextCursor: String(offset + opts.perPage), + nextCursor: `${opts.perPage}:${offset / opts.perPage + 1}:0`, }); }, ); @@ -537,8 +541,8 @@ describe("handleOrgAll", () => { Array.from({ length: 30 }, (_, i) => String(250 + i)), ); expect(listPaginated.mock.calls.map((call) => call[1])).toEqual([ - { cursor: "168", perPage: 84 }, - { cursor: "252", perPage: 84 }, + { cursor: "84:2:0", perPage: 84 }, + { cursor: "84:3:0", perPage: 84 }, ]); }); }); From ebc5b262897a564b1b3d7fdce51d59c92afa50ca Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 22:49:33 +0000 Subject: [PATCH 5/5] fix(cli): Write the mid-page skip in binary in is_prev A skip of up to ~99 rows took up to 100 zeros in is_prev. Python's int() also allows a single `_` between digits (PEP 515), so write the skip in binary with `_0` for 1 and `0` for 0 after a leading `0`: 82 becomes `0_00_000_00`. Sentry still reads it as is_prev=0. encodeSkip() documents the format. Co-Authored-By: BYK --- packages/cli/AGENTS.md | 8 +++- packages/cli/src/lib/api/infrastructure.ts | 47 ++++++++++++++----- .../cli/test/lib/api/infrastructure.test.ts | 9 ++-- .../cli/test/lib/api/project-dsns.test.ts | 5 +- .../cli/test/lib/paginate.property.test.ts | 4 +- 5 files changed, 51 insertions(+), 22 deletions(-) diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 61a227413..b4a94d28f 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -475,8 +475,12 @@ walk splits `--limit` evenly over the fewest requests (201 → 3 × 67). The cursors `paginate()` returns are Sentry cursors, so every CLI version and the API accept them, and stored `-c next` history works across upgrades and downgrades. When a call stops inside a page, the cursor points at that page -and records how many of its rows were already returned as extra zeros in -`is_prev` (`84:2:000` skips 2 rows; Sentry parses `is_prev` with `int()`). +and records how many of its rows were already returned in `is_prev`, in +binary with `_0` for 1 and `0` for 0 after a leading `0` (`84:2:0_00_000_00` +skips 82 = `0b1010010` rows). Sentry parses `is_prev` with Python's `int()`, +which reads any zeros joined by single underscores as 0, so it is still a +next-page cursor. `encodeSkip()` in `src/lib/api/infrastructure.ts` documents +the format. `paginate()` refetches the page and skips them; anything else that sends the cursor (with the page size it encodes, if any) gets those rows again, never fewer. Keep returned cursors valid Sentry diff --git a/packages/cli/src/lib/api/infrastructure.ts b/packages/cli/src/lib/api/infrastructure.ts index 4a49cbf6c..1699031c0 100644 --- a/packages/cli/src/lib/api/infrastructure.ts +++ b/packages/cli/src/lib/api/infrastructure.ts @@ -478,15 +478,10 @@ export async function autoPaginate( } /** - * A Sentry cursor, `::`, split before `is_prev`. - * - * Sentry parses `is_prev` with `int()` (`Cursor.from_string` in - * `src/sentry/utils/cursors.py`), so `0`, `00`, and `000` all mean "next". - * {@link paginate} uses the extra zeros to record how many rows of the page - * were already returned, which keeps its cursors valid Sentry cursors that - * any CLI version, or the API itself, accepts. + * A Sentry next-page cursor, `::`, split before + * `is_prev`, which may carry a skip count ({@link encodeSkip}). */ -const NEXT_CURSOR_RE = /^(.*:\d+):0(0*)$/; +const NEXT_CURSOR_RE = /^(.*:\d+):(0(?:_?0)*)$/; /** Resumes at the first row for every paginator the CLI uses. */ const FIRST_PAGE_CURSOR = "0:0:0"; @@ -512,6 +507,32 @@ type PagePosition = { skip: number; }; +/** + * Write how many rows of a page were already returned into a cursor's + * `is_prev` field, so the cursor stays a valid Sentry cursor. + * + * Sentry parses `is_prev` with Python's `int()` (`Cursor.from_string` in + * `src/sentry/utils/cursors.py`). `int()` ignores leading zeros and allows a + * single `_` between digits (PEP 515), so any string of `0`s joined by + * single `_`s is 0: "next page". The skip count is written in binary in that + * alphabet: a leading `0`, then `_0` for each 1 bit and `0` for each 0 bit, + * most significant first. 82 is `1010010`, so `0` + `_0 0 _0 0 0 _0 0` + * gives `0_00_000_00`. + * + * The API and older CLIs read the cursor as the page's own cursor and return + * the page from its first row; {@link decodeSkip} recovers the count. + * + * @param skip - Rows of the page already returned; must be at least 1 + */ +function encodeSkip(skip: number): string { + return `0${skip.toString(2).replaceAll("1", "_0")}`; +} + +/** Read the skip count {@link encodeSkip} wrote; plain `0` is no skip. */ +function decodeSkip(isPrev: string): number { + return Number.parseInt(isPrev.slice(1).replaceAll("_0", "1"), 2) || 0; +} + /** * Encode where a walk resumes as a Sentry cursor. A server cursor that is not * a Sentry next-page cursor cannot carry the skip, so it is returned as is and @@ -520,7 +541,7 @@ type PagePosition = { function encodePageCursor(cursor: string | undefined, skip: number): string { const server = cursor ?? FIRST_PAGE_CURSOR; const next = skip > 0 ? NEXT_CURSOR_RE.exec(server) : null; - return next ? `${next[1]}:${"0".repeat(skip + 1)}` : server; + return next ? `${next[1]}:${encodeSkip(skip)}` : server; } /** @@ -542,7 +563,7 @@ function decodePageCursor( const cursor = next ? `${next[1]}:0` : raw; const position = { cursor: cursor === FIRST_PAGE_CURSOR ? undefined : cursor, - skip: next?.[2]?.length ?? 0, + skip: next?.[2] ? decodeSkip(next[2]) : 0, }; const perPage = serverCursorPageSize(cursor); return perPage === undefined ? position : { ...position, perPage }; @@ -620,9 +641,9 @@ function pageSizeForLimit(limit: number): number { * the rows already returned. * * Returned cursors are Sentry cursors, so older CLI versions and the API - * accept them too; mid-page cursors add zeros to `is_prev` to record the rows - * to skip ({@link NEXT_CURSOR_RE}), which a caller that does not know about - * them sees again rather than misses (given the page size a page-number + * accept them too; mid-page cursors record the rows to skip in `is_prev` + * ({@link encodeSkip}), which a caller that does not know about them sees + * again rather than misses (given the page size a page-number * cursor encodes). Pass cursors back through this function * to resume exactly. Raw server cursors are accepted as a starting point. * diff --git a/packages/cli/test/lib/api/infrastructure.test.ts b/packages/cli/test/lib/api/infrastructure.test.ts index 90441997e..8e187c099 100644 --- a/packages/cli/test/lib/api/infrastructure.test.ts +++ b/packages/cli/test/lib/api/infrastructure.test.ts @@ -930,8 +930,9 @@ describe("paginate", () => { const first = await paginate({ limit: 250 }, spy); expect(first.data).toEqual(Array.from({ length: 250 }, (_, i) => i)); - // Page 84:2 was partly returned: 82 rows to skip, as zeros in is_prev. - expect(first.nextCursor).toBe(`84:2:${"0".repeat(83)}`); + // Page 84:2 was partly returned: 82 (0b1010010) rows to skip, in binary + // in is_prev with `_0` for 1 and `0` for 0. + expect(first.nextCursor).toBe("84:2:0_00_000_00"); const second = await paginate( { limit: 10, cursor: first.nextCursor }, spy, @@ -953,7 +954,7 @@ describe("paginate", () => { const spy = server((perPage, offset) => `0:${offset + perPage}:0`); const first = await paginate({ limit: 250 }, spy); - expect(first.nextCursor).toBe(`0:168:${"0".repeat(83)}`); + expect(first.nextCursor).toBe("0:168:0_00_000_00"); const second = await paginate( { limit: 10, cursor: first.nextCursor }, spy, @@ -972,7 +973,7 @@ describe("paginate", () => { const first = await paginate({ limit: 3 }, (perPage, cursor) => spy(perPage + 2, cursor), ); - expect(first.nextCursor).toBe("0:0:0000"); + expect(first.nextCursor).toBe("0:0:0_0_0"); const second = await paginate( { limit: 2, cursor: first.nextCursor }, spy, diff --git a/packages/cli/test/lib/api/project-dsns.test.ts b/packages/cli/test/lib/api/project-dsns.test.ts index 4e0f2693a..19ba613f2 100644 --- a/packages/cli/test/lib/api/project-dsns.test.ts +++ b/packages/cli/test/lib/api/project-dsns.test.ts @@ -164,8 +164,9 @@ describe.each([ const first = await list({ limit: 149, cursor: "0:25:0" }); expect(projectIds).toEqual(organization ? ["42"] : []); expect(first.data).toHaveLength(149); - // Page 0:100 was partly returned: 74 rows to skip, as zeros in is_prev. - expect(first.nextCursor).toBe(`0:100:${"0".repeat(75)}`); + // Page 0:100 was partly returned: 74 (0b1001010) rows to skip, in binary + // in is_prev. + expect(first.nextCursor).toBe("0:100:0_000_00_00"); const second = await list({ limit: 50, cursor: first.nextCursor }); expect( diff --git a/packages/cli/test/lib/paginate.property.test.ts b/packages/cli/test/lib/paginate.property.test.ts index 4a34c7c89..b5e74a3c5 100644 --- a/packages/cli/test/lib/paginate.property.test.ts +++ b/packages/cli/test/lib/paginate.property.test.ts @@ -147,7 +147,9 @@ describe.each(paginators)("property: paginate over $name", ({ make }) => { if (!result.nextCursor) { return; } - expect(result.nextCursor).toMatch(/^\d+:\d+:0+$/); + // is_prev must stay a Python int literal equal to 0: zeros joined by + // single underscores (PEP 515), which `Cursor.from_string` accepts. + expect(result.nextCursor).toMatch(/^\d+:\d+:0(?:_?0)*$/); const { data } = await fetchPage(perPage, result.nextCursor); const next = result.data.length; expect(data[0]).toBeLessThanOrEqual(next);