From 239b6356286afe5641d4aceb7e34ad7f7fa310d6 Mon Sep 17 00:00:00 2001 From: SAPHID Date: Mon, 12 Oct 2026 22:34:31 +0000 Subject: [PATCH 1/2] fix(bitbucket): advance pull request pages through tied updates Pass delivered boundary IDs to Bitbucket so timestamp continuations reach unseen pull requests before applying the page limit. Deduplicate IDs when serializing the next cursor. Co-Authored-By: GPT-6.1 Sol --- .../pullRequest/PullRequestService.test.ts | 92 ++++++++++++++++++- .../src/pullRequest/PullRequestService.ts | 17 ++-- .../server/BitbucketPullRequestApi.test.ts | 53 +++++++++++ .../src/server/BitbucketPullRequestApi.ts | 8 +- .../src/server/PullRequestProvider.ts | 5 + 5 files changed, 162 insertions(+), 13 deletions(-) diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 3850ed63a7b1..1845ffff0b57 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -34,6 +34,9 @@ import * as SourceControlProviderRegistry from "../sourceControl/SourceControlPr import * as SourceControlRateLimit from "@t3tools/source-control-core/server/SourceControlRateLimit"; import * as ForgejoCli from "@t3tools/source-control-forgejo/server/ForgejoCli"; import * as ForgejoPullRequestProvider from "@t3tools/source-control-forgejo/server/ForgejoPullRequestProvider"; +import * as BitbucketApi from "@t3tools/source-control-bitbucket/server/BitbucketApi"; +import * as BitbucketPullRequestApi from "@t3tools/source-control-bitbucket/server/BitbucketPullRequestApi"; +import * as BitbucketPullRequestProvider from "@t3tools/source-control-bitbucket/server/BitbucketPullRequestProvider"; import { PullRequestProviderError, type ProviderChangeRequest, @@ -885,6 +888,89 @@ it.effect("uses a provider's raw cursor advance when it consumed malformed rows" }), ); +it.effect("continues Bitbucket listings through tied timestamps and terminates", () => + Effect.gen(function* () { + const boundary = "2026-10-12T00:00:00.000Z"; + const older = "2026-10-11T00:00:00.000Z"; + const filters: string[] = []; + const rows = [1, 2, 3, 4, 5].map((id) => ({ + id, + title: `Pull request ${id}`, + state: "OPEN", + created_on: older, + updated_on: id <= 3 ? boundary : older, + source: { branch: { name: `feat/${id}` } }, + destination: { branch: { name: "main" } }, + links: { html: { href: `https://bitbucket.org/acme/web/pull-requests/${id}` } }, + })); + const provider = yield* BitbucketPullRequestProvider.make.pipe( + Effect.provide( + BitbucketPullRequestApi.layer.pipe( + Layer.provide( + Layer.mock(BitbucketApi.BitbucketApi)({ + request: ({ url }) => { + if (url === "/user") { + return Effect.succeed({ + body: JSON.stringify({ nickname: "bilal" }), + truncated: false, + }); + } + const filter = + new URL(url, "https://api.bitbucket.org").searchParams.get("q") ?? ""; + filters.push(filter); + const updatedBefore = /updated_on <= (\S+)/.exec(filter)?.[1]; + const excluded = new Set( + Array.from(filter.matchAll(/id != (\d+)/g), (match) => Number(match[1])), + ); + const values = rows.filter( + (row) => + (updatedBefore === undefined || row.updated_on <= updatedBefore) && + !excluded.has(row.id), + ); + return Effect.succeed({ body: JSON.stringify({ values }), truncated: false }); + }, + }), + ), + ), + ), + ); + const service = yield* makeService({ + projects: [ + project({ + id: "p1", + title: "web", + workspaceRoot: "/w", + repository: "acme/web", + provider: "bitbucket", + host: "bitbucket.org", + }), + ], + providers: [provider], + }); + const key = "bitbucket.org acme/web"; + const first = yield* service.list({ state: "open", limit: 2 }); + assert.deepStrictEqual( + first.entries.map((entry) => entry.number), + [1, 2], + ); + assert.deepStrictEqual(first.nextCursors, { [key]: `${boundary}|2|1,2` }); + const second = yield* service.list({ state: "open", limit: 2, cursors: first.nextCursors }); + assert.deepStrictEqual( + second.entries.map((entry) => entry.number), + [3, 4], + ); + assert.deepStrictEqual(second.nextCursors, { [key]: `${older}|4|4` }); + const third = yield* service.list({ state: "open", limit: 2, cursors: second.nextCursors }); + assert.deepStrictEqual( + third.entries.map((entry) => entry.number), + [5], + ); + assert.isFalse(third.truncated); + assert.deepStrictEqual(third.nextCursors, {}); + assert.strictEqual(filters.length, 3); + }), +); + it.effect("reads only the repositories it was asked to carry on with", () => Effect.gen(function* () { const listed: string[] = []; @@ -914,7 +1000,9 @@ it.effect("reads only the repositories it was asked to carry on with", () => // is here to avoid. The host summaries stay over the workspace, because the switcher they // fill is about the workspace rather than about this slice. assert.deepStrictEqual(listed, ["acme/web"]); - assert.deepStrictEqual(cursors, [{ updatedBefore: "2026-07-02T00:00:00Z", delivered: 99 }]); + assert.deepStrictEqual(cursors, [ + { updatedBefore: "2026-07-02T00:00:00Z", delivered: 99, seenAt: [7] }, + ]); assert.strictEqual(result.providers.length, 1); }), ); @@ -4378,7 +4466,7 @@ it.effect("carries a slice on without sending the rows it already sent", () => [4], ); assert.deepStrictEqual(result.nextCursors, { - "github.com acme/web": "2026-07-02T00:00:00Z|2|3,3,4", + "github.com acme/web": "2026-07-02T00:00:00Z|2|3,4", }); }), ); diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index d57934f10a16..baa1ad5afad8 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -419,7 +419,7 @@ interface RepositoryBatch { readonly nextCursor: string | null; } -/** What the providers are told, plus the part only the service acts on. */ +/** A decoded continuation, with boundary exclusions always present. */ interface ListCursor extends ProviderListCursor { /** * The rows already handed over at exactly `updatedBefore`. The next read asks for that instant @@ -499,11 +499,11 @@ function listCursorAt( fetched: ReadonlyArray, deliveredCount: number, ): string { - const seenAt = [ + const seenAt = new Set([ ...(previous?.updatedBefore === boundary ? previous.seenAt : []), ...fetched.filter((item) => item.updatedAt === boundary).map((item) => item.number), - ]; - return `${boundary}|${(previous?.delivered ?? 0) + deliveredCount}|${seenAt.join(",")}`; + ]); + return `${boundary}|${(previous?.delivered ?? 0) + deliveredCount}|${[...seenAt].join(",")}`; } /** A host that cannot be read at all, as opposed to one request that failed. */ @@ -1382,12 +1382,15 @@ export const make = Effect.gen(function* () { // answers unnarrowed rather than failing. query: input.query, filters: input.filters, - // Only the two fields a host can act on: which rows have already been sent at the - // boundary instant is this service's business, not a provider's. + // Timestamp-paged hosts exclude boundary rows before applying their page limit. ...(cursor === undefined ? {} : { - cursor: { updatedBefore: cursor.updatedBefore, delivered: cursor.delivered }, + cursor: { + updatedBefore: cursor.updatedBefore, + delivered: cursor.delivered, + seenAt: cursor.seenAt, + }, }), }) .pipe( diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts index 8d8feaba8150..ce2a34ddb656 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts @@ -85,6 +85,59 @@ afterEach(() => { }); layer("BitbucketPullRequestApi.layer", (it) => { + it.effect("advances through tied timestamps before reaching older pull requests", () => + Effect.gen(function* () { + const boundary = "2026-10-12T00:00:00.000Z"; + const older = "2026-10-11T00:00:00.000Z"; + const rows = [1, 2, 3, 4, 5].map((id) => ({ + id, + updated_on: id <= 3 ? boundary : older, + })); + mockedRequest.mockImplementation(({ url }) => { + const filter = new URL(url, "https://api.bitbucket.org").searchParams.get("q") ?? ""; + const updatedBefore = /updated_on <= (\S+)/.exec(filter)?.[1]; + const excluded = new Set( + Array.from(filter.matchAll(/id != (\d+)/g), (match) => Number(match[1])), + ); + const values = rows + .filter( + (row) => + (updatedBefore === undefined || row.updated_on <= updatedBefore) && + !excluded.has(row.id), + ) + .map((row) => JSON.parse(pullRequestJson(row))); + return Effect.succeed(response(valuePage(values))); + }); + const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; + const input = { repository: "acme/web", state: "open" as const, limit: 2 }; + const first = yield* api.listPullRequests(input); + assert.deepStrictEqual( + first.items.map((item) => item.number), + [1, 2], + ); + assert.isTrue(first.truncated); + const second = yield* api.listPullRequests({ + ...input, + cursor: { updatedBefore: boundary, delivered: 2, seenAt: [1, 2] }, + }); + assert.deepStrictEqual( + second.items.map((item) => item.number), + [3, 4], + ); + assert.isTrue(second.truncated); + const third = yield* api.listPullRequests({ + ...input, + cursor: { updatedBefore: older, delivered: 4, seenAt: [4] }, + }); + assert.deepStrictEqual( + third.items.map((item) => item.number), + [5], + ); + assert.isFalse(third.truncated); + assert.strictEqual(mockedRequest.mock.calls.length, 3); + }), + ); + it.effect("asks for reviewers, newest first, at Bitbucket's page ceiling", () => Effect.gen(function* () { mockedRequest.mockReturnValueOnce(Effect.succeed(response(page(3, 1)))); diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts index 6dd04a4a0773..88ca00dd153c 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts @@ -626,13 +626,13 @@ export const make = Effect.gen(function* () { withRepository(input.repository, (path) => { const search = input.query?.trim() ?? ""; // Both narrowings share the one `q` Bitbucket takes, so they are ANDed rather than one - // replacing the other. The boundary instant is read inclusively — the rows already sent - // at it come back and the caller drops them, which is what keeps their neighbours at the - // same instant from being skipped. A date is a bare literal in this grammar, and this one - // was checked against a timestamp's shape before it got here. + // replacing the other. Read the boundary inclusively, excluding the IDs already sent + // before the page limit is applied so unseen rows at the same instant remain reachable. + // Dates are bare literals here; the service validates the cursor's timestamp and IDs. const predicates = [ ...(search.length === 0 ? [] : [searchFilter(search)]), ...(input.cursor === undefined ? [] : [`updated_on <= ${input.cursor.updatedBefore}`]), + ...(input.cursor?.seenAt ?? []).map((number) => `id != ${number}`), ]; return listPage({ // Reviewers are not on a listing by default, and `viewerReviewRequested` needs them. diff --git a/packages/source-control-core/src/server/PullRequestProvider.ts b/packages/source-control-core/src/server/PullRequestProvider.ts index 85f5d1893ffb..53a4c3479dae 100644 --- a/packages/source-control-core/src/server/PullRequestProvider.ts +++ b/packages/source-control-core/src/server/PullRequestProvider.ts @@ -199,6 +199,11 @@ export interface ProviderListCursor { * count too when the provider reports a `cursorAdvance`. */ readonly delivered: number; + /** + * Rows already handed over at `updatedBefore`. Timestamp-paged hosts can exclude these before + * applying their limit so a full page of tied updates does not repeat the same prefix. + */ + readonly seenAt?: ReadonlyArray | undefined; } /** One repository's row inside an answer that spans several of them. */ From 2b5c6b77f8254e9401ab0db6f1d74bdce5435de2 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 08:44:23 +0000 Subject: [PATCH 2/2] test(bitbucket): keep one integrated tied-page regression --- .../pullRequest/PullRequestService.test.ts | 33 +++++------- .../server/BitbucketPullRequestApi.test.ts | 53 ------------------- 2 files changed, 13 insertions(+), 73 deletions(-) diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index e07ed1745f22..fdb182969386 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -928,7 +928,6 @@ it.effect("continues Bitbucket listings through tied timestamps and terminates", Effect.gen(function* () { const boundary = "2026-10-12T00:00:00.000Z"; const older = "2026-10-11T00:00:00.000Z"; - const filters: string[] = []; const rows = [1, 2, 3, 4, 5].map((id) => ({ id, title: `Pull request ${id}`, @@ -953,7 +952,6 @@ it.effect("continues Bitbucket listings through tied timestamps and terminates", } const filter = new URL(url, "https://api.bitbucket.org").searchParams.get("q") ?? ""; - filters.push(filter); const updatedBefore = /updated_on <= (\S+)/.exec(filter)?.[1]; const excluded = new Set( Array.from(filter.matchAll(/id != (\d+)/g), (match) => Number(match[1])), @@ -983,27 +981,22 @@ it.effect("continues Bitbucket listings through tied timestamps and terminates", ], providers: [provider], }); - const key = "bitbucket.org acme/web"; - const first = yield* service.list({ state: "open", limit: 2 }); + let page = yield* service.list({ state: "open", limit: 2 }); assert.deepStrictEqual( - first.entries.map((entry) => entry.number), + page.entries.map((entry) => entry.number), [1, 2], ); - assert.deepStrictEqual(first.nextCursors, { [key]: `${boundary}|2|1,2` }); - const second = yield* service.list({ state: "open", limit: 2, cursors: first.nextCursors }); - assert.deepStrictEqual( - second.entries.map((entry) => entry.number), - [3, 4], - ); - assert.deepStrictEqual(second.nextCursors, { [key]: `${older}|4|4` }); - const third = yield* service.list({ state: "open", limit: 2, cursors: second.nextCursors }); - assert.deepStrictEqual( - third.entries.map((entry) => entry.number), - [5], - ); - assert.isFalse(third.truncated); - assert.deepStrictEqual(third.nextCursors, {}); - assert.strictEqual(filters.length, 3); + for (const expected of [[3, 4], [5]]) { + assert.isTrue(page.truncated); + assert.isNotEmpty(page.nextCursors); + page = yield* service.list({ state: "open", limit: 2, cursors: page.nextCursors }); + assert.deepStrictEqual( + page.entries.map((entry) => entry.number), + expected, + ); + } + assert.isFalse(page.truncated); + assert.deepStrictEqual(page.nextCursors, {}); }), ); diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts index ff5dac5e4157..9132b936b9e7 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts @@ -80,59 +80,6 @@ afterEach(() => { }); layer("BitbucketPullRequestApi.layer", (it) => { - it.effect("advances through tied timestamps before reaching older pull requests", () => - Effect.gen(function* () { - const boundary = "2026-10-12T00:00:00.000Z"; - const older = "2026-10-11T00:00:00.000Z"; - const rows = [1, 2, 3, 4, 5].map((id) => ({ - id, - updated_on: id <= 3 ? boundary : older, - })); - mockedRequest.mockImplementation(({ url }) => { - const filter = new URL(url, "https://api.bitbucket.org").searchParams.get("q") ?? ""; - const updatedBefore = /updated_on <= (\S+)/.exec(filter)?.[1]; - const excluded = new Set( - Array.from(filter.matchAll(/id != (\d+)/g), (match) => Number(match[1])), - ); - const values = rows - .filter( - (row) => - (updatedBefore === undefined || row.updated_on <= updatedBefore) && - !excluded.has(row.id), - ) - .map((row) => JSON.parse(pullRequestJson(row))); - return Effect.succeed(response(valuePage(values))); - }); - const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; - const input = { repository: "acme/web", state: "open" as const, limit: 2 }; - const first = yield* api.listPullRequests(input); - assert.deepStrictEqual( - first.items.map((item) => item.number), - [1, 2], - ); - assert.isTrue(first.truncated); - const second = yield* api.listPullRequests({ - ...input, - cursor: { updatedBefore: boundary, delivered: 2, seenAt: [1, 2] }, - }); - assert.deepStrictEqual( - second.items.map((item) => item.number), - [3, 4], - ); - assert.isTrue(second.truncated); - const third = yield* api.listPullRequests({ - ...input, - cursor: { updatedBefore: older, delivered: 4, seenAt: [4] }, - }); - assert.deepStrictEqual( - third.items.map((item) => item.number), - [5], - ); - assert.isFalse(third.truncated); - assert.strictEqual(mockedRequest.mock.calls.length, 3); - }), - ); - it.effect("asks for reviewers, newest first, at Bitbucket's page ceiling", () => Effect.gen(function* () { mockedRequest.mockReturnValueOnce(Effect.succeed(response(page(3, 1))));