diff --git a/.server-changes/runs-backward-pagination-slice.md b/.server-changes/runs-backward-pagination-slice.md new file mode 100644 index 00000000000..41695f4e159 --- /dev/null +++ b/.server-changes/runs-backward-pagination-slice.md @@ -0,0 +1,14 @@ +--- +area: webapp +type: fix +--- + +Fix an off-by-one in `ClickHouseRunsRepository.listRunIds` backward pagination. +When paging backward with more rows before the page (`hasMore`), the displayed +page was sliced as `rows.slice(1, size + 1)`, which dropped the row closest to +the cursor and kept the extra "has-more" sentinel — returning a page that +straddled two logical pages (one row from the correct previous page plus one +from the page before it). The result set is always the first `page.size` rows +(the sentinel is the trailing element in both directions), so the slice is now +`rows.slice(0, size)` for forward and backward alike. Forward pagination and the +cursor values were already correct and are unchanged. diff --git a/apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts b/apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts index 473d3fc7685..fcf1c811d70 100644 --- a/apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts +++ b/apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts @@ -117,17 +117,24 @@ export class ClickHouseRunsRepository implements IRunsRepository { previousCursor = cursorFor(reversedRows.at(1)); nextCursor = cursorFor(reversedRows.at(options.page.size)); } else { - nextCursor = cursorFor(reversedRows.at(options.page.size - 1)); + // No newer rows, so there's no previous (newer) page. The next + // (older) cursor is the oldest row on this page = rows[0] (rows are + // ASC here). Index by the actual row count, not page.size — on a + // partial page (fewer than page.size rows) page.size-1 overshoots + // and would null the cursor, stranding forward navigation. + nextCursor = cursorFor(rows.at(0)); } break; } } - const runIds = ( - direction === "backward" && hasMore - ? rows.slice(1, options.page.size + 1) - : rows.slice(0, options.page.size) - ).map((row) => row.runId); + // The page is always the first `page.size` rows of the result. listRunRows + // fetches one extra row only to detect `hasMore`; that extra row is the + // farthest from the cursor in BOTH directions (forward orders DESC, backward + // orders ASC), so it's always the trailing element to drop — never the + // leading one. (Slicing `[1, size+1]` for backward dropped the row closest + // to the cursor and kept the has-more sentinel, straddling two pages.) + const runIds = rows.slice(0, options.page.size).map((row) => row.runId); return { runIds, pagination: { nextCursor, previousCursor } }; } diff --git a/apps/webapp/test/runsRepositoryCursor.test.ts b/apps/webapp/test/runsRepositoryCursor.test.ts index 854c59941bb..79677060f7d 100644 --- a/apps/webapp/test/runsRepositoryCursor.test.ts +++ b/apps/webapp/test/runsRepositoryCursor.test.ts @@ -296,4 +296,226 @@ describe("RunsRepository cursor pagination", () => { ]); } ); + + replicationContainerTest( + "backward pagination across multiple pages returns each page intact (no straddling)", + async ({ clickhouseContainer, redisOptions, postgresContainer, prisma }) => { + const { clickhouse } = await setupClickhouseReplication({ + prisma, + databaseUrl: postgresContainer.getConnectionUri(), + clickhouseUrl: clickhouseContainer.getConnectionUrl(), + redisOptions, + }); + + const organization = await prisma.organization.create({ + data: { title: "test", slug: "test" }, + }); + const project = await prisma.project.create({ + data: { + name: "test", + slug: "test", + organizationId: organization.id, + externalRef: "test", + }, + }); + const runtimeEnvironment = await prisma.runtimeEnvironment.create({ + data: { + slug: "test", + type: "DEVELOPMENT", + projectId: project.id, + organizationId: organization.id, + apiKey: "test", + pkApiKey: "test", + shortcode: "test", + }, + }); + + // Five runs so a backward page has more rows before it (hasMore === true), + // which is the case the off-by-one in the backward slice corrupts. + const ids = [ + "aaaaaaaaaaaaaaaaaaaaaaaa", + "bbbbbbbbbbbbbbbbbbbbbbbb", + "cccccccccccccccccccccccc", + "dddddddddddddddddddddddd", + "eeeeeeeeeeeeeeeeeeeeeeee", + ]; + const base = new Date("2026-06-04T16:55:07.000Z").getTime(); + for (let i = 0; i < ids.length; i++) { + await prisma.taskRun.create({ + data: { + id: ids[i], + createdAt: new Date(base + (ids.length - 1 - i) * 1000), + friendlyId: `run_${ids[i]}`, + taskIdentifier: "my-task", + payload: JSON.stringify({ foo: "bar" }), + traceId: `trace_${i}`, + spanId: `span_${i}`, + queue: "test", + runtimeEnvironmentId: runtimeEnvironment.id, + projectId: project.id, + organizationId: organization.id, + environmentType: "DEVELOPMENT", + engine: "V2", + }, + }); + } + + await setTimeout(1000); + + const runsRepository = new RunsRepository({ prisma, clickhouse }); + const baseOptions = { + projectId: project.id, + environmentId: runtimeEnvironment.id, + organizationId: organization.id, + }; + + const sortIds = (page: string[]) => page.slice().sort(); + + // Walk every forward page, capturing the run ids and the previousCursor. + const forwardPages: Array<{ ids: string[]; previousCursor: string | null }> = []; + let cursor: string | undefined = undefined; + for (let guard = 0; guard < 20; guard++) { + const page = await runsRepository.listRuns({ + ...baseOptions, + page: { size: 2, cursor, direction: cursor ? "forward" : undefined }, + }); + forwardPages.push({ + ids: page.runs.map((r) => r.id), + previousCursor: page.pagination.previousCursor, + }); + if (!page.pagination.nextCursor) break; + cursor = page.pagination.nextCursor; + } + + // Forward pagination itself must cover every run exactly once, in 3 pages. + expect(forwardPages.flatMap((p) => p.ids).sort()).toEqual(ids.slice().sort()); + expect(forwardPages).toHaveLength(3); + + // Walk backward from the last page. Each backward page must equal the + // corresponding forward page exactly — no row from an adjacent page + // bleeding in (the straddle bug returned e.g. {b,c} instead of {c,d}). + const backwardPages: string[][] = []; + let backCursor: string | null = forwardPages[forwardPages.length - 1].previousCursor; + for (let guard = 0; guard < 20 && backCursor; guard++) { + const page = await runsRepository.listRuns({ + ...baseOptions, + page: { size: 2, cursor: backCursor, direction: "backward" }, + }); + backwardPages.push(page.runs.map((r) => r.id)); + backCursor = page.pagination.previousCursor; + } + + const expectedBackward = forwardPages + .slice(0, -1) + .reverse() + .map((p) => sortIds(p.ids)); + expect(backwardPages.map(sortIds)).toEqual(expectedBackward); + + // And the full backward traversal (last page + everything walked back to + // the start) covers every run exactly once. + const seen = [...forwardPages[forwardPages.length - 1].ids, ...backwardPages.flat()]; + expect(seen.slice().sort()).toEqual(ids.slice().sort()); + expect(new Set(seen).size).toBe(ids.length); + } + ); + + replicationContainerTest( + "a partial backward page still exposes a forward cursor (no stranding)", + async ({ clickhouseContainer, redisOptions, postgresContainer, prisma }) => { + const { clickhouse } = await setupClickhouseReplication({ + prisma, + databaseUrl: postgresContainer.getConnectionUri(), + clickhouseUrl: clickhouseContainer.getConnectionUrl(), + redisOptions, + }); + + const organization = await prisma.organization.create({ + data: { title: "test", slug: "test" }, + }); + const project = await prisma.project.create({ + data: { + name: "test", + slug: "test", + organizationId: organization.id, + externalRef: "test", + }, + }); + const runtimeEnvironment = await prisma.runtimeEnvironment.create({ + data: { + slug: "test", + type: "DEVELOPMENT", + projectId: project.id, + organizationId: organization.id, + apiKey: "test", + pkApiKey: "test", + shortcode: "test", + }, + }); + + // Three runs; created_at descending order is [a, b, c] (a newest). + const ids = [ + "aaaaaaaaaaaaaaaaaaaaaaaa", + "bbbbbbbbbbbbbbbbbbbbbbbb", + "cccccccccccccccccccccccc", + ]; + const base = new Date("2026-06-04T16:55:07.000Z").getTime(); + for (let i = 0; i < ids.length; i++) { + await prisma.taskRun.create({ + data: { + id: ids[i], + createdAt: new Date(base + (ids.length - 1 - i) * 1000), + friendlyId: `run_${ids[i]}`, + taskIdentifier: "my-task", + payload: JSON.stringify({ foo: "bar" }), + traceId: `trace_${i}`, + spanId: `span_${i}`, + queue: "test", + runtimeEnvironmentId: runtimeEnvironment.id, + projectId: project.id, + organizationId: organization.id, + environmentType: "DEVELOPMENT", + engine: "V2", + }, + }); + } + + await setTimeout(1000); + + const runsRepository = new RunsRepository({ prisma, clickhouse }); + const baseOptions = { + projectId: project.id, + environmentId: runtimeEnvironment.id, + organizationId: organization.id, + }; + + // First page (size 2) = {a, b}; its nextCursor sits at b's boundary. + const first = await runsRepository.listRuns({ ...baseOptions, page: { size: 2 } }); + expect(first.runs.map((r) => r.id).sort()).toEqual([ + "aaaaaaaaaaaaaaaaaaaaaaaa", + "bbbbbbbbbbbbbbbbbbbbbbbb", + ]); + + // Paging backward from that cursor lands on a *partial* page — just the + // newest run {a}, with no rows before it (hasMore === false). + const back = await runsRepository.listRuns({ + ...baseOptions, + page: { size: 2, cursor: first.pagination.nextCursor!, direction: "backward" }, + }); + expect(back.runs.map((r) => r.id)).toEqual(["aaaaaaaaaaaaaaaaaaaaaaaa"]); + + // A partial backward page must still expose a forward cursor, or the user + // is stranded with no way to page back down. + expect(back.pagination.nextCursor).toBeTruthy(); + + // And paging forward from it reaches the remaining runs. + const forward = await runsRepository.listRuns({ + ...baseOptions, + page: { size: 2, cursor: back.pagination.nextCursor!, direction: "forward" }, + }); + expect(forward.runs.map((r) => r.id).sort()).toEqual([ + "bbbbbbbbbbbbbbbbbbbbbbbb", + "cccccccccccccccccccccccc", + ]); + } + ); });