Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 90 additions & 2 deletions apps/server/src/pullRequest/PullRequestService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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[] = [];
Expand Down Expand Up @@ -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);
}),
);
Expand Down Expand Up @@ -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",
});
}),
);
Expand Down
17 changes: 10 additions & 7 deletions apps/server/src/pullRequest/PullRequestService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -499,11 +499,11 @@ function listCursorAt(
fetched: ReadonlyArray<ProviderChangeRequest>,
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. */
Expand Down Expand Up @@ -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,
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
},
}),
})
.pipe(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<number> | undefined;
}

/** One repository's row inside an answer that spans several of them. */
Expand Down
Loading