From f00d5e7e1fd948609789dfb54217051a6c46f5b5 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Mon, 8 Jun 2026 13:04:55 +0100 Subject: [PATCH 1/2] fix(webapp): correct backward pagination slice in listRunIds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backward pagination dropped the wrong end of the result set. listRunRows fetches page.size + 1 rows to detect hasMore; that extra row is the one farthest from the cursor in both directions (forward orders DESC, backward orders ASC), so it is always the trailing element. Forward correctly sliced rows[0..size); backward+hasMore sliced rows[1..size+1], dropping the row closest to the cursor and keeping the has-more sentinel. The result was a page that straddled two logical pages (one run from the correct previous page plus one from the page before it) — so paging "newer" across a boundary repeated and skipped runs. Both directions now slice rows[0..size). Forward pagination, the hasMore=false backward path, and all cursor values were already correct and are unchanged. Adds a regression test that walks forward across multiple pages, then walks backward from the last page and asserts each backward page exactly reproduces the corresponding forward page (and the full traversal covers every run once). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../runs-backward-pagination-slice.md | 14 ++ .../clickhouseRunsRepository.server.ts | 12 +- apps/webapp/test/runsRepositoryCursor.test.ts | 122 ++++++++++++++++++ 3 files changed, 143 insertions(+), 5 deletions(-) create mode 100644 .server-changes/runs-backward-pagination-slice.md 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..04573d8bdb7 100644 --- a/apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts +++ b/apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts @@ -123,11 +123,13 @@ export class ClickHouseRunsRepository implements IRunsRepository { } } - 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..d02e79c2736 100644 --- a/apps/webapp/test/runsRepositoryCursor.test.ts +++ b/apps/webapp/test/runsRepositoryCursor.test.ts @@ -296,4 +296,126 @@ 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); + } + ); }); From 67e67888996eb9ee1191ac836b47d31d1fcb53a7 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Tue, 9 Jun 2026 09:57:13 +0100 Subject: [PATCH 2/2] fix(webapp): keep forward cursor on a partial backward page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In listRunIds' backward !hasMore branch, nextCursor was indexed at reversedRows[page.size - 1]. On a partial page (fewer than page.size rows — reachable via runs.list by passing a forward page's cursor as a backward cursor) that index overshoots and yields undefined, so nextCursor became null and forward navigation was stranded. Take the oldest row on the page (rows[0]) instead — equivalent for full pages, correct for partial ones. Adds a regression test: backward onto a partial first page must still expose a working forward cursor. --- .../clickhouseRunsRepository.server.ts | 7 +- apps/webapp/test/runsRepositoryCursor.test.ts | 100 ++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts b/apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts index 04573d8bdb7..fcf1c811d70 100644 --- a/apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts +++ b/apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts @@ -117,7 +117,12 @@ 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; } diff --git a/apps/webapp/test/runsRepositoryCursor.test.ts b/apps/webapp/test/runsRepositoryCursor.test.ts index d02e79c2736..79677060f7d 100644 --- a/apps/webapp/test/runsRepositoryCursor.test.ts +++ b/apps/webapp/test/runsRepositoryCursor.test.ts @@ -418,4 +418,104 @@ describe("RunsRepository cursor pagination", () => { 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", + ]); + } + ); });