diff --git a/apps/web/src/lib/provider-review/bitbucket-read.test.ts b/apps/web/src/lib/provider-review/bitbucket-read.test.ts index d04a286fda..b3cec1d6f2 100644 --- a/apps/web/src/lib/provider-review/bitbucket-read.test.ts +++ b/apps/web/src/lib/provider-review/bitbucket-read.test.ts @@ -1,4 +1,7 @@ import { describe, expect, it, beforeEach, afterEach } from '@jest/globals'; +import crypto from 'node:crypto'; +import { NEXTAUTH_SECRET } from '@kilocode/web-shared/lib/config.server'; +import { createSignedToken } from '@/lib/signed-token'; import { getMergeRestrictions, getPullRequest, @@ -8,6 +11,7 @@ import { listChecks, listDiscussions, listInbox, + INBOX_CURSOR_MAX_CHARS, } from './bitbucket-read'; import { BitbucketReviewError } from './bitbucket-authorization'; import { FILE_LINES_MAX } from '@/lib/github-pr-review/dtos'; @@ -24,6 +28,7 @@ jest.mock('@/lib/integrations/platforms/bitbucket/workspace-access-token-reposit jest.mock('@kilocode/web-shared/lib/config.server', () => ({ GIT_TOKEN_SERVICE_API_URL: 'https://token-service.example.com', + NEXTAUTH_SECRET: 'bitbucket-inbox-test-secret', })); jest.mock('@kilocode/web-shared/lib/tokens', () => ({ @@ -943,29 +948,94 @@ describe('listInbox', () => { destination: { branch: { name: 'main' }, repository: { full_name: fullName } }, }); - it('fans out over the integration repositories and carries full identity', async () => { + type InboxRow = { + id: number; + updated_on: string; + source: { repository: { full_name: string } }; + destination: { repository: { full_name: string } }; + }; + type InboxRequest = { slug: string; query: string | null; page: string | null }; + + /** A provider timestamp in milliseconds, including sub-millisecond digits. */ + const inboxTime = (value: string) => { + const ms = Date.parse(value); + const fraction = /\.(\d+)/.exec(value)?.[1] ?? ''; + return fraction.length > 3 ? ms + Number(fraction.slice(3)) / 10 ** (fraction.length - 3) : ms; + }; + + /** + * The workspace inbox as the provider serves it: each repository answers + * with its rows at or below the requested boundary, newest first, one page. + */ + const serveInbox = (rowsBySlug: Record, requests: InboxRequest[]) => { mockReadCachedRepositories.mockResolvedValue({ status: 'available', - repositories: [ - { - id: '87654321-4321-4321-4321-210987654321', - workspaceUuid: WORKSPACE.uuid, - name: 'repo', - fullName: 'acme/repo', - private: true, - defaultBranch: 'main', - }, - { - id: '99999999-8888-7777-6666-555555555555', - workspaceUuid: WORKSPACE.uuid, - name: 'empty-repo', - fullName: 'acme/empty-repo', - private: true, - defaultBranch: 'main', - }, - ], + repositories: Object.keys(rowsBySlug).map((slug, index) => ({ + id: `87654321-4321-4321-4321-${String(index).padStart(12, '0')}`, + workspaceUuid: WORKSPACE.uuid, + name: slug, + fullName: `acme/${slug}`, + private: true, + defaultBranch: 'main', + })), syncedAt: '2026-09-06T00:00:00.000Z', }); + fetchMock.mockImplementation(async (url: string | URL) => { + const full = url.toString(); + if (full.includes('token-service.example.com')) { + return jsonResponse({ status: 'available', token: 'at-mock-token', workspace: WORKSPACE }); + } + const parsed = new URL(full); + const match = parsed.pathname.match(/^\/2\.0\/repositories\/acme\/([^/]+)\/pullrequests$/); + const rows = match ? rowsBySlug[match[1]] : undefined; + if (!match || !rows) return jsonResponse({ pagelen: 50, values: [], next: null }); + const query = parsed.searchParams.get('q'); + const page = Number(parsed.searchParams.get('page') ?? '1'); + requests.push({ slug: match[1], query, page: String(page) }); + const boundary = query?.match(/^state="OPEN" AND updated_on <= (.+)$/); + const boundaryMs = boundary ? inboxTime(boundary[1]) : null; + const eligible = rows + .filter(row => boundaryMs === null || inboxTime(row.updated_on) <= boundaryMs) + .sort((left, right) => inboxTime(right.updated_on) - inboxTime(left.updated_on)); + return jsonResponse({ + pagelen: 50, + values: eligible.slice((page - 1) * 50, page * 50).map(row => ({ + ...row, + source: { repository: { full_name: `acme/${match[1]}` } }, + destination: { repository: { full_name: `acme/${match[1]}` } }, + })), + next: eligible.length > page * 50 ? 'https://api.bitbucket.org/2.0/next?page=2' : null, + }); + }); + }; + const minutesFixture = (firstId: number, count: number) => + Array.from({ length: count }, (_, index) => + inboxPr(firstId + index, `2026-09-02T00:${String(index).padStart(2, '0')}:00+00:00`) + ); + + /** The identity a continuation is bound to: the caller plus the repository set. */ + const inboxIdentity = (slugs: string[]) => + `bitbucket-inbox:acme:${ORG_OWNER.organizationId}:${ORG_OWNER.userId}:${crypto + .createHash('sha256') + .update( + [...slugs] + .sort() + .map(slug => `acme/${slug}`.toLowerCase()) + .join('\0') + ) + .digest('base64url')}`; + + /** A continuation token in the shape the read layer issues. */ + const inboxCursor = (identity: string, ceiling: string, pages = Buffer.alloc(2), window = 0) => + createSignedToken({ + kind: 'bitbucket-inbox-cursor', + identity, + ceiling, + window, + pages: pages.toString('base64url'), + }); + + it('fans out over the workspace repositories and carries full identity', async () => { fetchMock.mockImplementation(async (url: string | URL) => { const full = url.toString(); if (full.includes('token-service.example.com')) { @@ -1012,38 +1082,321 @@ describe('listInbox', () => { expect(result.nextCursor).toBeNull(); }); - it('reads only the integration repositories', async () => { + it('continues one repository from the signed ceiling', async () => { + // 60 open PRs: the first load serves the newest 50 and hands back a signed + // continuation; the next load re-reads the first page, skips the rows + // already served, and reads the second page in the same load. + const requests: InboxRequest[] = []; + serveInbox({ repo: minutesFixture(100, 60) }, requests); + + const first = await listInbox(ORG_OWNER); + expect(first.items).toHaveLength(50); + expect(first.items[0]?.ref).toMatchObject({ prId: 159 }); + expect(first.nextCursor).toBeTruthy(); + expect(requests).toHaveLength(1); + expect(requests[0]).toMatchObject({ slug: 'repo', query: 'state="OPEN"', page: '1' }); + + const second = await listInbox(ORG_OWNER, first.nextCursor!); + // The first load's page was fully served, so the continuation moves to the + // second provider page under the first load's ceiling. + expect(requests).toHaveLength(2); + expect(requests[1]).toMatchObject({ + slug: 'repo', + query: 'state="OPEN" AND updated_on <= 2026-09-02T00:59:00+00:00', + page: '2', + }); + expect(second.items).toHaveLength(10); + expect(second.items[0]?.ref).toMatchObject({ prId: 109 }); + expect(second.nextCursor).toBeNull(); + + const served = [...first.items, ...second.items].flatMap(item => + item.ref && 'prId' in item.ref ? [item.ref.prId] : [] + ); + expect(new Set(served).size).toBe(60); + }); + + it('keeps the ceiling precise enough for rows inside one millisecond', async () => { + // 51 rows share a millisecond and differ only below it: the window serves + // the newest 50, and the continuation must keep the last one, whose + // provider timestamp sits above a millisecond-truncated ceiling. + const requests: InboxRequest[] = []; + serveInbox( + { + repo: Array.from({ length: 51 }, (_, index) => + inboxPr( + 300 + index, + `2026-09-02T00:00:02.${String(410101 + index).padStart(6, '0')}+00:00` + ) + ), + }, + requests + ); + + const first = await listInbox(ORG_OWNER); + expect(first.items).toHaveLength(50); + expect(requests).toHaveLength(1); + + const second = await listInbox(ORG_OWNER, first.nextCursor!); + expect(first.nextCursor).toBeTruthy(); + expect(requests[1]).toMatchObject({ + query: 'state="OPEN" AND updated_on <= 2026-09-02T00:00:02.410151+00:00', + }); + expect( + second.items.map(item => (item.ref && 'prId' in item.ref ? item.ref.prId : null)) + ).toEqual([300]); + expect(second.nextCursor).toBeNull(); + }); + + it('advances past a provider page whose rows are all served', async () => { + const requests: InboxRequest[] = []; + const tiedAt = '2026-09-02T00:00:00.000000+00:00'; + serveInbox( + { repo: Array.from({ length: 51 }, (_, index) => inboxPr(500 + index, tiedAt)) }, + requests + ); + + const first = await listInbox(ORG_OWNER); + expect(first.items).toHaveLength(50); + expect(first.nextCursor).toBeTruthy(); + expect(first.nextCursor!.length).toBeLessThanOrEqual(2048); + + const second = await listInbox(ORG_OWNER, first.nextCursor!); + expect( + second.items.map(item => (item.ref && 'prId' in item.ref ? item.ref.prId : null)) + ).toEqual([550]); + expect(second.nextCursor).toBeNull(); + expect(requests.map(request => request.page)).toEqual(['1', '2']); + }); + it('keeps equal-time rows in a stable order across inbox pages', async () => { + const requests: InboxRequest[] = []; + const tiedAt = '2026-09-02T00:00:00.000000+00:00'; + serveInbox( + { + 'repo-a': Array.from({ length: 40 }, (_, index) => + inboxPr(1000 + index, tiedAt, 'acme/repo-a') + ), + 'repo-b': Array.from({ length: 40 }, (_, index) => + inboxPr(2000 + index, tiedAt, 'acme/repo-b') + ), + 'repo-c': Array.from({ length: 40 }, (_, index) => + inboxPr(3000 + index, tiedAt, 'acme/repo-c') + ), + }, + requests + ); + + const first = await listInbox(ORG_OWNER); + const second = await listInbox(ORG_OWNER, first.nextCursor!); + const third = await listInbox(ORG_OWNER, second.nextCursor!); + const ids = [...first.items, ...second.items, ...third.items].map(item => + item.ref && 'prId' in item.ref ? item.ref.prId : null + ); + + expect(ids).toHaveLength(120); + expect(new Set(ids).size).toBe(120); + expect(ids).toEqual([ + ...Array.from({ length: 40 }, (_, index) => 1000 + index), + ...Array.from({ length: 40 }, (_, index) => 2000 + index), + ...Array.from({ length: 40 }, (_, index) => 3000 + index), + ]); + expect(requests.every(request => request.page === '1')).toBe(true); + expect(third.nextCursor).toBeNull(); + }); + + it('continues several repositories across pages', async () => { + // repo-a's rows are all older than repo-b's, so the first load serves + // exactly repo-b's page and the continuation serves repo-a's. + const requests: InboxRequest[] = []; + serveInbox( + { + 'repo-a': minutesFixture(100, 50), + 'repo-b': Array.from({ length: 50 }, (_, index) => + inboxPr(200 + index, `2026-09-02T01:${String(index).padStart(2, '0')}:00+00:00`) + ), + }, + requests + ); + + const first = await listInbox(ORG_OWNER); + expect(first.items).toHaveLength(50); + expect(first.items[0]?.ref).toMatchObject({ prId: 249 }); + expect(first.nextCursor).toBeTruthy(); + + const second = await listInbox(ORG_OWNER, first.nextCursor!); + expect(second.items).toHaveLength(50); + expect(second.items[0]?.ref).toMatchObject({ prId: 149 }); + expect(second.nextCursor).toBeNull(); + // repo-b has nothing left, so the continuation reads only repo-a. + expect(requests).toHaveLength(3); + expect(requests[2]).toMatchObject({ + slug: 'repo-a', + query: 'state="OPEN" AND updated_on <= 2026-09-02T01:49:00+00:00', + page: '1', + }); + + const served = [...first.items, ...second.items].flatMap(item => + item.ref && 'prId' in item.ref ? [item.ref.prId] : [] + ); + expect(new Set(served).size).toBe(100); + }); + + it('serves every row once when one repository spans provider pages and another is older', async () => { + // repo-a has 100 rows with distinct times, two provider pages of 50; + // repo-b has one row older than every repo-a row. Every load reads one + // provider page per repository, so repo-b's single row must not drag the + // continuation below repo-a's second page. + const base = Date.UTC(2026, 8, 2, 0, 0, 0); + const requests: InboxRequest[] = []; + serveInbox( + { + 'repo-a': Array.from({ length: 100 }, (_, index) => + inboxPr(100 + index, new Date(base - index * 1000).toISOString()) + ), + 'repo-b': [inboxPr(900, new Date(base - 1000 * 1000).toISOString())], + }, + requests + ); + + const served: number[] = []; + let cursor: string | undefined; + for (let load = 0; load < 10; load += 1) { + const page = await listInbox(ORG_OWNER, cursor); + served.push( + ...page.items.flatMap(item => (item.ref && 'prId' in item.ref ? [item.ref.prId] : [])) + ); + cursor = page.nextCursor ?? undefined; + if (cursor === undefined) break; + } + + expect(new Set(served).size).toBe(101); + expect(served).toHaveLength(101); + }); + + it('serves every tied row when another repository also sits at the boundary', async () => { + // 60 tied rows in one repository and one tied row in a repository that + // sorts after it: the later repository's row must not hide the rows that + // continue onto the first repository's second provider page. + const requests: InboxRequest[] = []; + const tiedAt = '2026-09-02T00:00:00.000000+00:00'; + serveInbox( + { + 'repo-a': Array.from({ length: 60 }, (_, index) => + inboxPr(100 + index, tiedAt, 'acme/repo-a') + ), + 'repo-b': [inboxPr(900, tiedAt, 'acme/repo-b')], + }, + requests + ); + + const served: number[] = []; + let cursor: string | undefined; + for (let load = 0; load < 4; load += 1) { + const page = await listInbox(ORG_OWNER, cursor); + served.push( + ...page.items.flatMap(item => (item.ref && 'prId' in item.ref ? [item.ref.prId] : [])) + ); + cursor = page.nextCursor ?? undefined; + if (cursor === undefined) break; + } + + expect(served).toHaveLength(61); + expect(new Set(served).size).toBe(61); + expect(served).toEqual([...Array.from({ length: 60 }, (_, index) => 100 + index), 900]); + expect(requests).toHaveLength(4); + }); + + it('ends a repository whose final provider page omits the next link', async () => { + // Both repositories share one timestamp. The first has a single extra row + // on its last page, which Bitbucket answers without a `next` key, while the + // second keeps paginating: the finished repository must not be asked for a + // page that does not exist. + const tiedAt = '2026-09-02T00:00:00.000000+00:00'; + const rowsBySlug: Record = { + 'repo-a': Array.from({ length: 51 }, (_, index) => + inboxPr(100 + index, tiedAt, 'acme/repo-a') + ), + 'repo-b': Array.from({ length: 100 }, (_, index) => + inboxPr(500 + index, tiedAt, 'acme/repo-b') + ), + }; + mockReadCachedRepositories.mockResolvedValue({ + status: 'available', + repositories: Object.keys(rowsBySlug).map((slug, index) => ({ + id: `87654321-4321-4321-4321-${String(index).padStart(12, '0')}`, + workspaceUuid: WORKSPACE.uuid, + name: slug, + fullName: `acme/${slug}`, + private: true, + defaultBranch: 'main', + })), + syncedAt: '2026-09-06T00:00:00.000Z', + }); + const requests: InboxRequest[] = []; fetchMock.mockImplementation(async (url: string | URL) => { const full = url.toString(); if (full.includes('token-service.example.com')) { return jsonResponse({ status: 'available', token: 'at-mock-token', workspace: WORKSPACE }); } const parsed = new URL(full); - if (parsed.pathname === '/2.0/repositories/acme/repo/pullrequests') { - return jsonResponse({ - pagelen: 50, - values: [inboxPr(12, '2026-09-02T00:00:00.000000+00:00')], - next: null, - }); - } - throw new Error(`Unexpected provider read: ${parsed.pathname}`); + const match = parsed.pathname.match(/^\/2\.0\/repositories\/acme\/([^/]+)\/pullrequests$/); + const rows = match ? rowsBySlug[match[1]] : undefined; + if (!match || !rows) return jsonResponse({ pagelen: 50, values: [] }); + const page = Number(parsed.searchParams.get('page') ?? '1'); + requests.push({ slug: match[1], query: parsed.searchParams.get('q'), page: String(page) }); + const values = rows.slice((page - 1) * 50, page * 50); + if (values.length === 0) { + // What Bitbucket answers for a page past the collection. + return jsonResponse( + { type: 'error', error: { message: 'Page number does not exist' } }, + 404 + ); + } + return jsonResponse( + rows.length > page * 50 + ? { pagelen: 50, values, next: 'https://api.bitbucket.org/2.0/next?page=2' } + : { pagelen: 50, values } + ); }); - const result = await listInbox(ORG_OWNER); + const served: number[] = []; + let cursor: string | undefined; + for (let load = 0; load < 6; load += 1) { + const page = await listInbox(ORG_OWNER, cursor); + served.push( + ...page.items.flatMap(item => (item.ref && 'prId' in item.ref ? [item.ref.prId] : [])) + ); + cursor = page.nextCursor ?? undefined; + if (cursor === undefined) break; + } - expect(result.items.map(item => item.ref)).toEqual([ - expect.objectContaining({ workspace: 'acme', repoSlug: 'repo', prId: 12 }), - ]); - expect(fetchMock.mock.calls.map(call => new URL(String(call[0])).pathname)).toEqual([ - '/internal/bitbucket/workspace-access-token', - '/2.0/repositories/acme/repo/pullrequests', - ]); + expect(new Set(served).size).toBe(151); + expect(served).toHaveLength(151); + expect(cursor).toBeUndefined(); + // A page past either repository's last page answers 404: the walk must + // never ask for one, so the omitted `next` link — not the provider error — + // is what ends a repository. + for (const slug of ['repo-a', 'repo-b']) { + const pages = requests.filter(request => request.slug === slug).map(request => request.page); + expect(new Set(pages)).toEqual(new Set(['1', '2'])); + } }); - it('answers an empty page when the integration repository set is empty', async () => { + it('serves the rest of the inbox when a cached repository is no longer served', async () => { + // The cached set can name a repository the provider has deleted since its + // last sync: its collection answers 404, which ends that repository only. + const rowsBySlug: Record = { + 'repo-a': minutesFixture(100, 2), + }; mockReadCachedRepositories.mockResolvedValue({ status: 'available', - repositories: [], + repositories: ['repo-a', 'repo-gone'].map((slug, index) => ({ + id: `87654321-4321-4321-4321-${String(index).padStart(12, '0')}`, + workspaceUuid: WORKSPACE.uuid, + name: slug, + fullName: `acme/${slug}`, + private: true, + defaultBranch: 'main', + })), syncedAt: '2026-09-06T00:00:00.000Z', }); fetchMock.mockImplementation(async (url: string | URL) => { @@ -1051,220 +1404,483 @@ describe('listInbox', () => { if (full.includes('token-service.example.com')) { return jsonResponse({ status: 'available', token: 'at-mock-token', workspace: WORKSPACE }); } - throw new Error(`Unexpected provider read: ${full}`); + const parsed = new URL(full); + const match = parsed.pathname.match(/^\/2\.0\/repositories\/acme\/([^/]+)\/pullrequests$/); + const rows = match ? rowsBySlug[match[1]] : undefined; + if (!match || !rows) { + return jsonResponse({ type: 'error', error: { message: 'Repository not found' } }, 404); + } + return jsonResponse({ + pagelen: 50, + values: rows.map(row => ({ + ...row, + source: { repository: { full_name: `acme/${match[1]}` } }, + destination: { repository: { full_name: `acme/${match[1]}` } }, + })), + next: null, + }); }); - const result = await listInbox(ORG_OWNER); + const page = await listInbox(ORG_OWNER); - expect(result).toEqual({ items: [], nextCursor: null }); + expect(page.items.map(item => item.ref && 'prId' in item.ref && item.ref.prId)).toEqual([ + 101, 100, + ]); + expect(page.nextCursor).toBeNull(); }); - it('maps an unreadable repository set onto the review error kinds', async () => { - mockReadCachedRepositories.mockResolvedValue({ status: 'reconnect_required' }); - - await expect(listInbox(ORG_OWNER)).rejects.toMatchObject({ - kind: 'reconnect', - retryable: false, + it('ends a repository whose continuation page no longer exists', async () => { + // Load one is promised a second page of repo-b. Twenty of repo-b's rows + // leave the collection before load two, so the page it was promised is now + // past the end and answers 404: the remaining repositories must still page, + // and repo-b must not be asked again. + const rowsBySlug: Record = { + 'repo-a': Array.from({ length: 60 }, (_, index) => + inboxPr(100 + index, `2026-09-02T00:${String(59 - index).padStart(2, '0')}:00+00:00`) + ), + 'repo-b': Array.from({ length: 60 }, (_, index) => + inboxPr(200 + index, `2026-09-02T01:${String(59 - index).padStart(2, '0')}:00+00:00`) + ), + }; + const requests: InboxRequest[] = []; + mockReadCachedRepositories.mockResolvedValue({ + status: 'available', + repositories: Object.keys(rowsBySlug).map((slug, index) => ({ + id: `87654321-4321-4321-4321-${String(index).padStart(12, '0')}`, + workspaceUuid: WORKSPACE.uuid, + name: slug, + fullName: `acme/${slug}`, + private: true, + defaultBranch: 'main', + })), + syncedAt: '2026-09-06T00:00:00.000Z', + }); + fetchMock.mockImplementation(async (url: string | URL) => { + const full = url.toString(); + if (full.includes('token-service.example.com')) { + return jsonResponse({ status: 'available', token: 'at-mock-token', workspace: WORKSPACE }); + } + const parsed = new URL(full); + const match = parsed.pathname.match(/^\/2\.0\/repositories\/acme\/([^/]+)\/pullrequests$/); + const rows = match ? rowsBySlug[match[1]] : undefined; + if (!match || !rows) return jsonResponse({ pagelen: 50, values: [], next: null }); + const page = Number(parsed.searchParams.get('page') ?? '1'); + requests.push({ slug: match[1], query: parsed.searchParams.get('q'), page: String(page) }); + const values = rows.slice((page - 1) * 50, page * 50); + if (values.length === 0) { + return jsonResponse( + { type: 'error', error: { message: 'Page number does not exist' } }, + 404 + ); + } + return jsonResponse({ + pagelen: 50, + values: values.map(row => ({ + ...row, + source: { repository: { full_name: `acme/${match[1]}` } }, + destination: { repository: { full_name: `acme/${match[1]}` } }, + })), + next: rows.length > page * 50 ? 'https://api.bitbucket.org/2.0/next?page=2' : null, + }); }); - expect(fetchMock).not.toHaveBeenCalled(); + + const served: number[] = []; + let cursor: string | undefined; + for (let load = 0; load < 5; load += 1) { + const page = await listInbox(ORG_OWNER, cursor); + served.push( + ...page.items.flatMap(item => (item.ref && 'prId' in item.ref ? [item.ref.prId] : [])) + ); + cursor = page.nextCursor ?? undefined; + if (load === 0) { + // Load one served repo-b's newest 50 rows and was promised page two. + expect(served).toHaveLength(50); + expect(requests.filter(request => request.slug === 'repo-b')).toHaveLength(1); + rowsBySlug['repo-b'] = rowsBySlug['repo-b'].slice(0, 40); + } + if (cursor === undefined) break; + } + + expect(cursor).toBeUndefined(); + expect(served).toHaveLength(110); + expect(new Set(served).size).toBe(110); + expect(served.slice(0, 50)).toEqual(Array.from({ length: 50 }, (_, index) => 200 + index)); + expect(served.slice(50)).toEqual(Array.from({ length: 60 }, (_, index) => 100 + index)); + expect(requests.filter(request => request.slug === 'repo-b')).toHaveLength(2); }); - it('merges pages across repositories newest first and continues with a page cursor', async () => { + it('leaves a row without a comparable timestamp out of the walk', async () => { + // The provider can answer a row with no `updated_on`. It has no place in + // the merged newest-first order and no comparable position for a + // continuation, so it is left out while every orderable row is served. + const rows = Array.from({ length: 60 }, (_, index) => + inboxPr(100 + index, `2026-09-02T00:${String(59 - index).padStart(2, '0')}:00+00:00`) + ); + const values = [ + ...rows.slice(0, 50), + { ...inboxPr(900, ''), updated_on: null }, + ...rows.slice(50), + ]; + mockReadCachedRepositories.mockResolvedValue({ + status: 'available', + repositories: [ + { + id: '87654321-4321-4321-4321-000000000000', + workspaceUuid: WORKSPACE.uuid, + name: 'repo', + fullName: 'acme/repo', + private: true, + defaultBranch: 'main', + }, + ], + syncedAt: '2026-09-06T00:00:00.000Z', + }); fetchMock.mockImplementation(async (url: string | URL) => { const full = url.toString(); if (full.includes('token-service.example.com')) { return jsonResponse({ status: 'available', token: 'at-mock-token', workspace: WORKSPACE }); } const parsed = new URL(full); - if (parsed.pathname === '/2.0/repositories/acme/repo/pullrequests') { - // The provider listing is sorted newest first (the inbox request pins - // sort=-updated_on): page 2 holds only older rows. - const page = parsed.searchParams.get('page'); - if (page === '2') { - return jsonResponse({ - pagelen: 50, - values: [inboxPr(99, '2026-09-01T00:00:00.000000+00:00')], - next: null, - }); - } - return jsonResponse({ - pagelen: 50, - values: Array.from({ length: 50 }, (_, index) => - inboxPr(100 + index, `2026-09-02T00:00:${String(index).padStart(2, '0')}+00:00`) - ), - next: null, - }); - } - return jsonResponse({ pagelen: 50, values: [], next: null }); + const page = Number(parsed.searchParams.get('page') ?? '1'); + return jsonResponse( + values.length > page * 50 + ? { + pagelen: 50, + values: values.slice((page - 1) * 50, page * 50), + next: 'https://api.bitbucket.org/2.0/next?page=2', + } + : { pagelen: 50, values: values.slice((page - 1) * 50, page * 50) } + ); }); - const first = await listInbox(ORG_OWNER); - expect(first.items).toHaveLength(50); - expect(first.items[0]?.ref).toMatchObject({ prId: 149 }); - expect(first.nextCursor).toBeTruthy(); + const served: number[] = []; + let cursor: string | undefined; + for (let load = 0; load < 4; load += 1) { + const page = await listInbox(ORG_OWNER, cursor); + served.push( + ...page.items.flatMap(item => (item.ref && 'prId' in item.ref ? [item.ref.prId] : [])) + ); + cursor = page.nextCursor ?? undefined; + if (cursor === undefined) break; + } - const second = await listInbox(ORG_OWNER, first.nextCursor!); - // The second page continues the sorted sequence with the rows the first - // page's trim could not serve, instead of re-sorting a fresh fan-out. - expect(second.items.map(item => item.ref)).toEqual([expect.objectContaining({ prId: 99 })]); - expect(second.nextCursor).toBeNull(); + expect(served).toEqual(Array.from({ length: 60 }, (_, index) => 100 + index)); + expect(cursor).toBeUndefined(); }); - it('serves every row across pages: the sorted window refetches earlier provider pages', async () => { - // 60 PRs in one repository: a full first provider page (50) plus a - // 10-row older tail. Page 1 serves the newest 50; page 2 must serve the - // remaining 10 — none dropped, none duplicated. + it('starts each repository window with its own ceiling', async () => { + // The repository in the second window carries the newest row of the + // workspace. Its rows were never read under the first window's ceiling, so + // that ceiling must not filter them out. + const slugs = Array.from( + { length: 301 }, + (_, index) => `repo-${String(index).padStart(3, '0')}` + ); + const rowsBySlug: Record = {}; + for (const slug of slugs) { + rowsBySlug[slug] = [ + inboxPr( + slugs.indexOf(slug) + 1, + slug === 'repo-300' ? '2026-09-03T00:00:00+00:00' : '2026-09-02T00:00:00+00:00', + `acme/${slug}` + ), + ]; + } + const requests: InboxRequest[] = []; + serveInbox(rowsBySlug, requests); + + const served: number[] = []; + let cursor: string | undefined; + for (let load = 0; load < 12; load += 1) { + const page = await listInbox(ORG_OWNER, cursor); + served.push( + ...page.items.flatMap(item => (item.ref && 'prId' in item.ref ? [item.ref.prId] : [])) + ); + cursor = page.nextCursor ?? undefined; + if (cursor === undefined) break; + } + + expect(cursor).toBeUndefined(); + expect(new Set(served)).toEqual(new Set(Array.from({ length: 301 }, (_, index) => index + 1))); + expect(served).toHaveLength(301); + // The second window is read without the first window's ceiling. + const secondWindowRequest = requests.at(-1); + expect(secondWindowRequest).toMatchObject({ + slug: 'repo-300', + query: 'state="OPEN"', + page: '1', + }); + }); + + it('hands the walk to the next window when a window serves nothing', async () => { + // The first window holds repositories the provider no longer serves, so it + // produces no rows and no ceiling; the repository behind it still has to be + // read. + const slugs = Array.from( + { length: 301 }, + (_, index) => `repo-${String(index).padStart(3, '0')}` + ); + mockReadCachedRepositories.mockResolvedValue({ + status: 'available', + repositories: slugs.map((slug, index) => ({ + id: `87654321-4321-4321-4321-${String(index).padStart(12, '0')}`, + workspaceUuid: WORKSPACE.uuid, + name: slug, + fullName: `acme/${slug}`, + private: true, + defaultBranch: 'main', + })), + syncedAt: '2026-09-06T00:00:00.000Z', + }); + const requests: InboxRequest[] = []; fetchMock.mockImplementation(async (url: string | URL) => { const full = url.toString(); if (full.includes('token-service.example.com')) { return jsonResponse({ status: 'available', token: 'at-mock-token', workspace: WORKSPACE }); } const parsed = new URL(full); - if (parsed.pathname === '/2.0/repositories/acme/repo/pullrequests') { - const page = parsed.searchParams.get('page'); - if (page === '2') { - return jsonResponse({ - pagelen: 50, - values: Array.from({ length: 10 }, (_, index) => - inboxPr(150 + index, `2026-09-01T00:00:${String(index).padStart(2, '0')}+00:00`) - ), - next: null, - }); - } - return jsonResponse({ - pagelen: 50, - values: Array.from({ length: 50 }, (_, index) => - inboxPr(100 + index, `2026-09-02T00:00:${String(index).padStart(2, '0')}+00:00`) - ), - next: null, - }); + const match = parsed.pathname.match(/^\/2\.0\/repositories\/acme\/([^/]+)\/pullrequests$/); + requests.push({ + slug: match?.[1] ?? '', + query: parsed.searchParams.get('q'), + page: parsed.searchParams.get('page'), + }); + if (match?.[1] !== 'repo-300') { + return jsonResponse({ type: 'error', error: { message: 'Repository not found' } }, 404); } - return jsonResponse({ pagelen: 50, values: [], next: null }); + return jsonResponse({ + pagelen: 1, + values: [inboxPr(999, '2026-09-02T00:00:00+00:00', 'acme/repo-300')], + next: null, + }); }); const first = await listInbox(ORG_OWNER); - expect(first.items).toHaveLength(50); - expect(first.items[0]?.ref).toMatchObject({ prId: 149 }); + expect(first.items).toEqual([]); + expect(first.nextCursor).toBeTruthy(); const second = await listInbox(ORG_OWNER, first.nextCursor!); - expect(second.items).toHaveLength(10); - expect(second.items[0]?.ref).toMatchObject({ prId: 159 }); + expect(second.items).toHaveLength(1); + expect(second.items[0].ref).toMatchObject({ repoSlug: 'repo-300', prId: 999 }); expect(second.nextCursor).toBeNull(); - - const servedIds = new Set( - [...first.items, ...second.items].map(item => - item.ref && 'prId' in item.ref ? item.ref.prId : null - ) - ); - expect(servedIds.size).toBe(60); - for (let id = 100; id <= 159; id += 1) { - expect(servedIds.has(id)).toBe(true); - } + expect(requests.at(-1)).toMatchObject({ slug: 'repo-300', query: 'state="OPEN"' }); }); - it('serves every row of several repositories across pages without drops', async () => { - // Two integrated repositories with 50 open PRs each: page 1 serves the - // newest 50 of the 100 aggregated rows, page 2 the remaining 50 — the old - // page-N fan-out re-fetched provider page 2 (empty here) and dropped 50 - // rows. + it('continues a window whose pages carry no orderable row', async () => { + // Pages one and two answer rows without a timestamp and promise another + // page; page three holds the first orderable row. The walk must continue + // without a ceiling instead of ending at the first unorderable page. mockReadCachedRepositories.mockResolvedValue({ status: 'available', repositories: [ { - id: '87654321-4321-4321-4321-210987654321', - workspaceUuid: WORKSPACE.uuid, - name: 'repo-a', - fullName: 'acme/repo-a', - private: true, - defaultBranch: 'main', - }, - { - id: '99999999-8888-7777-6666-555555555555', + id: '87654321-4321-4321-4321-000000000000', workspaceUuid: WORKSPACE.uuid, - name: 'repo-b', - fullName: 'acme/repo-b', + name: 'repo', + fullName: 'acme/repo', private: true, defaultBranch: 'main', }, ], syncedAt: '2026-09-06T00:00:00.000Z', }); + const requests: InboxRequest[] = []; fetchMock.mockImplementation(async (url: string | URL) => { const full = url.toString(); if (full.includes('token-service.example.com')) { return jsonResponse({ status: 'available', token: 'at-mock-token', workspace: WORKSPACE }); } const parsed = new URL(full); - const match = parsed.pathname.match( - /^\/2\.0\/repositories\/acme\/(repo-[ab])\/pullrequests$/ - ); - if (match && parsed.searchParams.get('page') === '1') { - // repo-a rows are older than repo-b rows, so the first inbox page is - // exactly repo-b's page; the single provider page holds all 50 rows. - const base = match[1] === 'repo-a' ? 100 : 200; - const hour = match[1] === 'repo-a' ? '00' : '01'; - const fullName = `acme/${match[1]}`; + const page = Number(parsed.searchParams.get('page') ?? '1'); + requests.push({ slug: 'repo', query: parsed.searchParams.get('q'), page: String(page) }); + if (page < 3) { return jsonResponse({ - pagelen: 50, - values: Array.from({ length: 50 }, (_, index) => - inboxPr( - base + index, - `2026-09-02T${hour}:00:${String(index).padStart(2, '0')}.000000+00:00`, - fullName - ) - ), - next: null, + pagelen: 1, + values: [{ ...inboxPr(900 + page, ''), updated_on: null }], + next: `https://api.bitbucket.org/2.0/repositories/acme/repo/pullrequests?page=${page + 1}`, }); } - return jsonResponse({ pagelen: 50, values: [], next: null }); + return jsonResponse({ pagelen: 1, values: [inboxPr(999, '2026-09-02T00:00:00+00:00')] }); }); + const first = await listInbox(ORG_OWNER); + expect(first.items).toEqual([]); + expect(first.nextCursor).toBeTruthy(); + + const second = await listInbox(ORG_OWNER, first.nextCursor!); + expect(second.items.map(item => item.ref)).toEqual([ + { platform: 'bitbucket', workspace: 'acme', repoSlug: 'repo', prId: 999 }, + ]); + expect(second.nextCursor).toBeNull(); + expect(requests.at(-1)).toMatchObject({ slug: 'repo', query: 'state="OPEN"', page: '3' }); + }); + + it('reads one provider page per repository that still has rows', async () => { + // 12 repositories × 5 open PRs: the first load reads one page per + // repository, and the continuation reads only the two with rows left. + const rowsBySlug: Record = {}; + for (let repo = 0; repo < 12; repo += 1) { + rowsBySlug[`repo-${repo}`] = Array.from({ length: 5 }, (_, index) => + inboxPr( + 1000 + repo * 10 + index, + `2026-09-02T00:${String(repo * 5 + index).padStart(2, '0')}:00+00:00` + ) + ); + } + const requests: InboxRequest[] = []; + serveInbox(rowsBySlug, requests); + const first = await listInbox(ORG_OWNER); expect(first.items).toHaveLength(50); - expect(first.items[0]?.ref).toMatchObject({ prId: 249 }); expect(first.nextCursor).toBeTruthy(); + expect(requests).toHaveLength(12); const second = await listInbox(ORG_OWNER, first.nextCursor!); - expect(second.items).toHaveLength(50); - expect(second.items[0]?.ref).toMatchObject({ prId: 149 }); + expect(requests).toHaveLength(14); + expect(second.items).toHaveLength(10); expect(second.nextCursor).toBeNull(); + expect(requests.every(request => request.page === '1')).toBe(true); + expect(requests.filter(request => request.query === 'state="OPEN"')).toHaveLength(12); + }); - const servedIds = new Set( - [...first.items, ...second.items].map(item => - item.ref && 'prId' in item.ref ? item.ref.prId : null - ) + it('walks every cached repository across repository windows', async () => { + // One more repository than a cursor carries: the first window reads the + // first 300, and the continuation moves to the next window instead of the + // inbox stopping at the first slice. + const slugs = Array.from({ length: 301 }, (_, repo) => `repo-${repo}`); + const rowsBySlug: Record = {}; + for (const slug of slugs) { + rowsBySlug[slug] = [inboxPr(slugs.indexOf(slug) + 1, '2026-09-02T00:00:00+00:00')]; + } + const requests: InboxRequest[] = []; + serveInbox(rowsBySlug, requests); + + const first = await listInbox(ORG_OWNER); + const sorted = [...slugs].sort(); + expect([...new Set(requests.map(request => request.slug))].sort()).toEqual( + sorted.slice(0, 300) ); - expect(servedIds.size).toBe(100); - for (let id = 100; id <= 249; id += 1) { - if ((id >= 100 && id <= 149) || (id >= 200 && id <= 249)) { - expect(servedIds.has(id)).toBe(true); - } + expect(first.items).toHaveLength(50); + expect(first.nextCursor).toBeTruthy(); + + const served = first.items.flatMap(item => + item.ref && 'prId' in item.ref ? [item.ref.prId] : [] + ); + let cursor: string | undefined = first.nextCursor!; + for (let load = 0; load < 12 && cursor; load += 1) { + const page = await listInbox(ORG_OWNER, cursor); + served.push( + ...page.items.flatMap(item => (item.ref && 'prId' in item.ref ? [item.ref.prId] : [])) + ); + cursor = page.nextCursor ?? undefined; } + + // Every repository is served, including the one past the first window. + expect(new Set(served)).toEqual(new Set(Array.from({ length: 301 }, (_, index) => index + 1))); + expect(served).toHaveLength(301); + expect(new Set(requests.map(request => request.slug))).toEqual(new Set(sorted)); + // One row per repository: every load re-reads page one of the repositories + // in its window, and no repository is read past its first page. + expect(requests.every(request => request.page === '1')).toBe(true); }); - it('ignores a cursor minted for another workspace', async () => { - fetchMock.mockImplementation(async (url: string | URL) => { - const full = url.toString(); - if (full.includes('token-service.example.com')) { - return jsonResponse({ status: 'available', token: 'at-mock-token', workspace: WORKSPACE }); - } - const parsed = new URL(full); - if (parsed.pathname === '/2.0/repositories/acme/repo/pullrequests') { - expect(parsed.searchParams.get('page')).toBe('1'); - return jsonResponse({ - pagelen: 50, - values: [inboxPr(12, '2026-09-02T00:00:00.000000+00:00')], - next: null, - }); - } - return jsonResponse({ pagelen: 50, values: [], next: null }); + it('keeps a maximum-size continuation within the receiving wire limit', async () => { + const slugs = Array.from( + { length: 300 }, + (_, index) => `repo-${String(index).padStart(3, '0')}` + ); + const tiedAt = '2026-09-02T00:00:00.000000+00:00'; + const rowsBySlug: Record = {}; + for (const slug of slugs) { + rowsBySlug[slug] = Array.from({ length: 60 }, (_, index) => + inboxPr(index + 1, tiedAt, `acme/${slug}`) + ); + } + const requests: InboxRequest[] = []; + serveInbox(rowsBySlug, requests); + + const pages = Buffer.alloc(slugs.length * 2); + for (let index = 0; index < slugs.length; index += 1) pages.writeUInt16BE(2, index * 2); + const cursor = inboxCursor(inboxIdentity(slugs), tiedAt, pages); + expect(cursor.length).toBeLessThanOrEqual(INBOX_CURSOR_MAX_CHARS); + + const page = await listInbox(ORG_OWNER, cursor); + + expect(requests).toHaveLength(300); + expect(requests.every(request => request.page === '2')).toBe(true); + expect(page.nextCursor?.length).toBeLessThanOrEqual(INBOX_CURSOR_MAX_CHARS); + }); + + it('starts again at the first page when continuation state is not valid', async () => { + const requests: InboxRequest[] = []; + serveInbox({ repo: minutesFixture(100, 60) }, requests); + + const clientValue = Buffer.from( + JSON.stringify({ identity: 'bitbucket-inbox:acme', page: 10 }) + ).toString('base64url'); + const page = await listInbox(ORG_OWNER, clientValue); + + expect(requests).toHaveLength(1); + expect(requests[0]).toMatchObject({ query: 'state="OPEN"', page: '1' }); + expect(page.items).toHaveLength(50); + }); + + it('scopes continuation state to the workspace it was issued for', async () => { + const requests: InboxRequest[] = []; + serveInbox({ repo: minutesFixture(100, 60) }, requests); + + const foreign = createSignedToken({ + kind: 'bitbucket-inbox-cursor', + identity: 'bitbucket-inbox:evil', + ceiling: '2026-09-02T00:10:00.000+00:00', + window: 0, + pages: Buffer.alloc(2).toString('base64url'), }); + const page = await listInbox(ORG_OWNER, foreign); + + expect(requests).toHaveLength(1); + expect(requests[0]).toMatchObject({ query: 'state="OPEN"' }); + expect(page.items).toHaveLength(50); + }); + + it('accepts continuation state only with a matching signature', async () => { + const requests: InboxRequest[] = []; + serveInbox({ repo: minutesFixture(100, 60) }, requests); + const first = await listInbox(ORG_OWNER); + const cursor = first.nextCursor!; + requests.length = 0; - const foreign = Buffer.from( - JSON.stringify({ identity: 'bitbucket-inbox:evil', page: 7 }) + const invalidSignature = `${cursor.slice(0, -1)}${cursor.endsWith('A') ? 'B' : 'A'}`; + await listInbox(ORG_OWNER, invalidSignature); + + expect(requests).toHaveLength(1); + expect(requests[0]).toMatchObject({ query: 'state="OPEN"', page: '1' }); + }); + + it('keeps continuation state usable for one hour', async () => { + const requests: InboxRequest[] = []; + serveInbox({ repo: minutesFixture(100, 60) }, requests); + + const encoded = Buffer.from( + JSON.stringify({ + kind: 'bitbucket-inbox-cursor', + identity: inboxIdentity(['repo']), + ceiling: '2026-09-02T00:10:00.000+00:00', + window: 0, + pages: Buffer.alloc(2).toString('base64url'), + iat: Math.floor(Date.now() / 1000) - 2 * 60 * 60, + nonce: 'expired-nonce', + }) ).toString('base64url'); - const result = await listInbox(ORG_OWNER, foreign); - expect(result.items).toHaveLength(1); + const signature = crypto + .createHmac('sha256', NEXTAUTH_SECRET) + .update(encoded) + .digest('base64url'); + const page = await listInbox(ORG_OWNER, `${encoded}.${signature}`); + + expect(requests).toHaveLength(1); + expect(requests[0]).toMatchObject({ query: 'state="OPEN"' }); + expect(page.items).toHaveLength(50); }); it('reports a provider permission refusal as a reconnect, never as the caller permission', async () => { diff --git a/apps/web/src/lib/provider-review/bitbucket-read.ts b/apps/web/src/lib/provider-review/bitbucket-read.ts index cb243f1d49..ef55d5c800 100644 --- a/apps/web/src/lib/provider-review/bitbucket-read.ts +++ b/apps/web/src/lib/provider-review/bitbucket-read.ts @@ -11,17 +11,18 @@ */ import 'server-only'; +import { createHash } from 'node:crypto'; import { z } from 'zod'; -import type { - ProviderPrChecksResult, - ProviderPrFile, - ProviderPrFilesPage, - ProviderPrInboxItem, - ProviderPrInboxPage, - ProviderPrMergeBlockedReason, - ProviderPrMergeState, - ProviderPrSummary, - ProviderPrThread, +import { + type ProviderPrChecksResult, + type ProviderPrFile, + type ProviderPrFilesPage, + type ProviderPrInboxItem, + type ProviderPrInboxPage, + type ProviderPrMergeBlockedReason, + type ProviderPrMergeState, + type ProviderPrSummary, + type ProviderPrThread, } from '@kilocode/app-shared/provider-review'; import { authorizeRepository, @@ -30,9 +31,11 @@ import { BitbucketApiStatusError, BitbucketReviewError, type BitbucketRepositoryAccess, + type BitbucketWorkspaceRepositoryAccess, type BitbucketReviewOwner, } from './bitbucket-authorization'; import { FILE_LINES_MAX } from '@/lib/github-pr-review/dtos'; +import { createSignedToken, verifySignedToken } from '@/lib/signed-token'; const BITBUCKET_API_ORIGIN = 'https://api.bitbucket.org'; const BITBUCKET_PAGE_SIZE = 50; @@ -49,13 +52,18 @@ const MAX_SUMMARY_DIFFSTAT_PAGES = 3; const MAX_BUILD_PAGES = 3; /** How many repository PR collections the inbox fetches at once. */ const INBOX_REPOSITORY_CONCURRENCY = 8; +/** The maximum repository count served by the inbox on one load. */ +const MAX_INBOX_REPOSITORIES = 300; /** - * The provider pages one inbox page may walk per repository: inbox page N - * refetches provider pages 1..N so the sorted windows continue each other, - * and this bound keeps that fan-out capped (a bounded inbox beats an - * unbounded crawl). + * The receiving wire limit of an inbox cursor: the `pageCursor` bound the + * provider-review router accepts, so a cursor this layer issues always fits + * the next request. The per-repository page state is two bytes each — 300 + * repositories stay far below the limit — and a test pins the maximum-size + * cursor to it. */ -const MAX_INBOX_PROVIDER_PAGES = 10; +export const INBOX_CURSOR_MAX_CHARS = 2048; +/** How long a server-issued inbox cursor stays usable. */ +const INBOX_CURSOR_TTL_SECONDS = 60 * 60; /** * The task-collection page bound for the discussion task walk: the same * bounded walk the write layer's thread resolution uses, so one discussion @@ -972,13 +980,27 @@ function classifyInboxError(error: unknown): BitbucketReviewError { * PRs of the integration's repositories, newest first. * * The fan-out sorts across repositories, so one inbox page cannot map onto a - * single provider page: page N refetches provider pages 1..N of every - * integration repository (each listing is pinned to `sort=-updated_on`, the - * inbox's own sort key) and serves the sorted window - * [(N-1)·size, N·size). Refetching the earlier provider pages is what keeps - * the windows from drifting: a plain provider-page-N fan-out plus - * sort-and-trim would silently drop the rows page 1's trim left over, and - * they would never be re-served. + * single provider page. A signed continuation carries the provider ceiling of + * the window it is walking — the newest `updated_on` that window's first load + * saw, so every row of the window that existed then stays inside the + * collection — and, per repository, the provider page it is on and how many + * leading rows of that page are already served. + * + * Rows are emitted in the merged order, so a repository's served rows are + * always the leading rows of its current page: the next load re-reads that + * page, skips them, and moves to the next page once the page is fully served. + * The continuation advances monotonically, never moves back to the first page, + * and reads at most one provider page per repository per load (two when the + * current page is already fully served). A workspace with more cached + * repositories than one cursor can carry is walked one window of repositories + * at a time, so every cached repository is covered instead of the walk stopping + * at the first window; each window starts its own ceiling, because its rows + * were never read under the previous window's. A pull request updated after + * the window's first load sits above the ceiling and appears on a fresh inbox + * load; one that stops being open leaves the collection and shifts the rows + * after it up, which the provider's page-number paging cannot prevent. A + * repository or a page the provider no longer serves ends that repository's + * walk rather than failing the whole inbox. */ export async function listInbox( owner: BitbucketReviewOwner, @@ -986,119 +1008,347 @@ export async function listInbox( ): Promise { try { const access = await authorizeWorkspace(owner); - const identity = `bitbucket-inbox:${access.workspace.slug}`; - const page = decodeInboxPageCursor(cursor, identity); - if (page > MAX_INBOX_PROVIDER_PAGES) { - // Past the walk bound the pagination ends instead of crawling. - return { items: [], nextCursor: null }; - } - const slugs = access.repositories.map(repository => repository.slug); - if (slugs.length === 0) { - // An integration whose repository set is empty lists nothing: an empty - // page, no provider fan-out. - return { items: [], nextCursor: null }; - } - const allowedFullNames = new Set( - access.repositories.map(repository => repository.fullName.toLowerCase()) + const cachedRepositories = [...access.repositories].sort((left, right) => + left.fullName.toLowerCase() < right.fullName.toLowerCase() + ? -1 + : left.fullName.toLowerCase() > right.fullName.toLowerCase() + ? 1 + : 0 ); - const values: unknown[] = []; - let deeperProviderPage = false; - for (let providerPage = 1; providerPage <= page; providerPage += 1) { - let lastRequestedPageFull = false; - for (let offset = 0; offset < slugs.length; offset += INBOX_REPOSITORY_CONCURRENCY) { - const batch = slugs.slice(offset, offset + INBOX_REPOSITORY_CONCURRENCY); - const pages = await Promise.all( - batch.map(slug => - requestBitbucketJson( - access, - `/2.0/repositories/${encodeURIComponent(access.workspace.slug)}/${encodeURIComponent(slug)}/pullrequests`, - { - query: { - pagelen: BITBUCKET_PAGE_SIZE, - page: providerPage, - q: 'state="OPEN"', - sort: '-updated_on', - }, + const repositoryFingerprint = createHash('sha256') + .update(cachedRepositories.map(repository => repository.fullName.toLowerCase()).join('\0')) + .digest('base64url'); + const identity = + owner.type === 'organization' + ? `bitbucket-inbox:${access.workspace.slug}:${owner.organizationId}:${owner.userId}:${repositoryFingerprint}` + : `bitbucket-inbox:${access.workspace.slug}:user:${owner.userId}:${repositoryFingerprint}`; + const position = decodeInboxCursor(cursor, identity, cachedRepositories); + const windowIndex = position?.window ?? 0; + const repositories = inboxRepositoryWindow(cachedRepositories, windowIndex); + const repositoryNames = new Set( + repositories.map(repository => repository.fullName.toLowerCase()) + ); + const providerQuery = position?.ceiling + ? `state="OPEN" AND updated_on <= ${position.ceiling}` + : 'state="OPEN"'; + let window: InboxCandidate[] = []; + const pageReads: InboxPageRead[] = []; + for (let offset = 0; offset < repositories.length; offset += INBOX_REPOSITORY_CONCURRENCY) { + const batch = repositories + .slice(offset, offset + INBOX_REPOSITORY_CONCURRENCY) + .filter(repository => position?.pages.get(repository.fullName.toLowerCase())?.page !== 0); + const pages = await Promise.all( + batch.map(async repository => { + const key = repository.fullName.toLowerCase(); + let page = position?.pages.get(key)?.page ?? 1; + let served = position?.pages.get(key)?.served ?? 0; + for (let reads = 1; ; reads += 1) { + let payload: unknown; + try { + payload = await requestBitbucketJson( + access, + `/2.0/repositories/${encodeURIComponent(access.workspace.slug)}/${encodeURIComponent(repository.slug)}/pullrequests`, + { + query: { + pagelen: BITBUCKET_PAGE_SIZE, + page, + q: providerQuery, + sort: '-updated_on', + }, + } + ); + } catch (error) { + // A repository the cached set still names but the provider no + // longer serves, and a page an earlier load was promised for a + // collection that has since shrunk, are both that repository's + // end: the rest of the inbox still loads, and the repository + // stops appearing in later continuations. + if (classifyBitbucketError(error).kind === 'not_found') { + return { key, page, rows: [], fromRow: 0, hadNext: false }; } - ) - ) - ); - for (const payload of pages) { - const parsedPage = BitbucketPageSchema.safeParse(payload); - if (!parsedPage.success) { - throw new BitbucketReviewError('retryable', 'Bitbucket returned an unexpected page.'); + throw error; + } + const parsedPage = BitbucketPageSchema.safeParse(payload); + if (!parsedPage.success) { + throw new BitbucketReviewError('retryable', 'Bitbucket returned an unexpected page.'); + } + const rows = inboxItemsFrom(parsedPage.data.values, repositoryNames).map( + (item, rowIndex) => ({ item, repoKey: key, rowIndex }) + ); + const hadNext = Boolean(parsedPage.data.next); + // The page was fully served on an earlier load: move on to the next + // page within this load, so the continuation makes progress without + // spending a load on a page that has nothing left to offer. + if ( + served >= rows.length && + hadNext && + page < INBOX_PAGE_MAX && + reads < INBOX_PAGE_READS_PER_LOAD + ) { + page += 1; + served = 0; + continue; + } + return { key, page, rows, fromRow: Math.min(served, rows.length), hadNext }; } - values.push(...parsedPage.data.values); - if (parsedPage.data.values.length >= BITBUCKET_PAGE_SIZE) lastRequestedPageFull = true; - } + }) + ); + for (const read of pages) { + const merged = [...window, ...read.rows.slice(read.fromRow)].sort(compareInboxCandidates); + window = merged.slice(0, BITBUCKET_PAGE_SIZE); + pageReads.push(read); } - if (providerPage === page) deeperProviderPage = lastRequestedPageFull; } - const items: ProviderPrInboxItem[] = []; - for (const value of values) { - const parsed = BitbucketInboxPullRequestSchema.safeParse(value); - if (!parsed.success) continue; - const ref = inboxRefFrom(parsed.data, allowedFullNames); - if (!ref) continue; - items.push({ - ref, - title: parsed.data.title, - author: mapUser(parsed.data.author ?? null), - state: mapPullRequestState(parsed.data.state), - draft: parsed.data.draft === true, - updatedAt: parsed.data.updated_on ?? '', - }); + const ceiling = position?.ceiling ?? inboxCeiling(pageReads); + const emitted = new Map(); + for (const candidate of window) { + emitted.set(candidate.repoKey, (emitted.get(candidate.repoKey) ?? 0) + 1); + } + const pages = new Map(); + for (const repository of repositories) { + const carried = position?.pages.get(repository.fullName.toLowerCase()); + if (!carried || carried.page === 0) pages.set(repository.fullName.toLowerCase(), DONE); + } + for (const read of pageReads) { + const served = read.fromRow + (emitted.get(read.key) ?? 0); + pages.set( + read.key, + served >= read.rows.length + ? read.hadNext && read.page < INBOX_PAGE_MAX + ? { page: read.page + 1, served: 0 } + : DONE + : { page: read.page, served } + ); + } + const hasMore = [...pages.values()].some(state => state.page !== 0); + const nextWindow = windowIndex + 1; + const carriesNextWindow = + !hasMore && inboxRepositoryWindow(cachedRepositories, nextWindow).length > 0; + let nextCursor: string | null = null; + if (hasMore) { + // A page whose rows all lack a timestamp leaves no ceiling yet; the + // continuation then carries none and the next load reads on unfiltered, + // so the rest of the window is still walked instead of ending here. + nextCursor = encodeInboxCursor(identity, windowIndex, ceiling, pages, repositories); + } else if (carriesNextWindow) { + // This window is served in full: the walk moves to the next slice of the + // cached repositories, so a workspace larger than one window is still + // covered. The next window sets its own ceiling from its first load: + // its rows were never read under this window's ceiling, and one newer + // than it would otherwise stay outside the collection for the whole walk. + // A window that served nothing (every repository gone or empty) still + // hands the walk on, so the windows behind it are read. + nextCursor = encodeInboxCursor( + identity, + nextWindow, + null, + new Map(), + inboxRepositoryWindow(cachedRepositories, nextWindow) + ); } - items.sort((left, right) => inboxUpdatedMs(right) - inboxUpdatedMs(left)); - const skip = (page - 1) * BITBUCKET_PAGE_SIZE; - const window = items.slice(skip, skip + BITBUCKET_PAGE_SIZE); - // More follows when a repository still holds a page after the deepest - // one read, or when the sorted aggregate itself already reaches past - // this window. - const hasMore = deeperProviderPage || items.length > skip + window.length; return { - items: window, - nextCursor: - hasMore && page < MAX_INBOX_PROVIDER_PAGES - ? encodeInboxPageCursor(identity, page + 1) - : null, + items: window.map(candidate => candidate.item), + nextCursor, }; } catch (error) { throw classifyInboxError(error); } } +/** The row's `updated_on` in milliseconds, including any sub-millisecond digits. */ function inboxUpdatedMs(item: ProviderPrInboxItem): number { const ms = Date.parse(item.updatedAt); - return Number.isNaN(ms) ? 0 : ms; + if (Number.isNaN(ms)) return 0; + const fraction = /\.(\d+)/.exec(item.updatedAt)?.[1] ?? ''; + return fraction.length > 3 ? ms + Number(fraction.slice(3)) / 10 ** (fraction.length - 3) : ms; } /** - * The inbox cursor is a plain page counter, not a provider `next` URL: one - * inbox page fans out over the integration's repositories, so no single next - * link can represent it. The page number is the sorted window index — page N - * serves rows [(N-1)·size, N·size) of the merged newest-first order. A - * cursor minted for another workspace, or in the old next-URL shape, decodes - * to page 1 — a cursor can never switch the workspace a request reads. + * The newest provider timestamp the pages of a load carry: the continuation's + * fixed ceiling. Stored as the provider's own string, so the filter keeps the + * full microsecond precision the collection was read at. */ -function encodeInboxPageCursor(identity: string, page: number): string { - return Buffer.from(JSON.stringify({ identity, page })).toString('base64url'); +function inboxCeiling(reads: InboxPageRead[]): string | null { + let ceiling: string | null = null; + let newest = -Infinity; + for (const read of reads) { + for (const candidate of read.rows) { + if (Number.isNaN(Date.parse(candidate.item.updatedAt))) continue; + const ms = inboxUpdatedMs(candidate.item); + if (ms > newest) { + newest = ms; + ceiling = candidate.item.updatedAt; + } + } + } + return ceiling; } -function decodeInboxPageCursor(cursor: string | undefined, identity: string): number { - if (!cursor) return 1; - try { - const parsed = JSON.parse(Buffer.from(cursor, 'base64url').toString('utf8')) as { - identity?: unknown; - page?: unknown; - }; - if (parsed.identity !== identity || !Number.isInteger(parsed.page)) return 1; - return Math.max(1, parsed.page as number); - } catch { - return 1; +/** + * The inbox rows of one provider page, in provider order. A row the provider + * gives without a parseable `updated_on` has no place in the merged newest-first + * order and no comparable position for a continuation, so it is left out + * instead of sorting as the oldest row. + */ +function inboxItemsFrom( + values: unknown[], + allowedFullNames: ReadonlySet +): ProviderPrInboxItem[] { + const items: ProviderPrInboxItem[] = []; + for (const value of values) { + const parsed = BitbucketInboxPullRequestSchema.safeParse(value); + if (!parsed.success) continue; + const ref = inboxRefFrom(parsed.data, allowedFullNames); + if (ref?.platform !== 'bitbucket') continue; + const updatedAt = parsed.data.updated_on ?? ''; + if (Number.isNaN(Date.parse(updatedAt))) continue; + items.push({ + ref, + title: parsed.data.title, + author: mapUser(parsed.data.author ?? null), + state: mapPullRequestState(parsed.data.state), + draft: parsed.data.draft === true, + updatedAt, + }); + } + return items; +} + +/** + * The merged inbox order: newest first, ties broken by repository and then by + * the row's position in that repository's provider page. Page order is kept, + * so the rows a continuation served from a page are always its leading rows. + */ +function compareInboxCandidates(left: InboxCandidate, right: InboxCandidate): number { + const timestampOrder = inboxUpdatedMs(right.item) - inboxUpdatedMs(left.item); + if (timestampOrder !== 0) return timestampOrder; + if (left.repoKey !== right.repoKey) return left.repoKey < right.repoKey ? -1 : 1; + return left.rowIndex - right.rowIndex; +} + +const INBOX_CURSOR_KIND = 'bitbucket-inbox-cursor'; +/** Bits of the packed word that hold the page number; the rest hold the count. */ +const INBOX_PAGE_BITS = 10; +const INBOX_PAGE_MAX = (1 << INBOX_PAGE_BITS) - 1; +/** + * Provider page reads one repository may spend on a single load: its current + * page, plus the next page when the current one is already fully served. + */ +const INBOX_PAGE_READS_PER_LOAD = 2; +const inboxCursorPayloadSchema = z.object({ + kind: z.literal(INBOX_CURSOR_KIND), + identity: z.string().min(1), + /** The current window's ceiling; null starts a window whose walk begins now. */ + ceiling: z.string().min(1).nullable(), + window: z.number().int().min(0), + pages: z.string().max(800), +}); + +/** One provider row with the repository and page position it was read at. */ +type InboxCandidate = { + item: ProviderPrInboxItem; + repoKey: string; + rowIndex: number; +}; + +type InboxPageRead = { + key: string; + page: number; + rows: InboxCandidate[]; + /** How many of this page's leading rows were already served before this load. */ + fromRow: number; + hadNext: boolean; +}; + +/** + * One repository's position in the continuation: the provider page it is on + * and how many of that page's leading rows are served. Page 0 means the + * repository has nothing left to serve. + */ +type InboxPageState = { page: number; served: number }; + +/** A repository with nothing left to serve. */ +const DONE: InboxPageState = { page: 0, served: 0 }; + +type InboxCursorPosition = { + ceiling: string | null; + window: number; + pages: ReadonlyMap; +}; + +/** + * The slice of the sorted cached repositories one walk covers: the cursor's + * window index times the per-window bound. A workspace with more cached + * repositories than one window is walked window by window, so every cached + * repository is served within the cursor's wire limit. + */ +function inboxRepositoryWindow( + repositories: BitbucketWorkspaceRepositoryAccess['repositories'], + window: number +): BitbucketWorkspaceRepositoryAccess['repositories'] { + const start = window * MAX_INBOX_REPOSITORIES; + return repositories.slice(start, start + MAX_INBOX_REPOSITORIES); +} + +/** + * The inbox cursor is a signed continuation token carrying the caller and + * repository-set identity, the provider timestamp ceiling the collection was + * read at, the repository window the walk is on, and two packed bytes per + * repository of that window: its page number and how many leading rows of that + * page are served. + */ +function encodeInboxCursor( + identity: string, + window: number, + ceiling: string | null, + pages: ReadonlyMap, + repositories: BitbucketWorkspaceRepositoryAccess['repositories'] +): string { + const pageState = Buffer.alloc(repositories.length * 2); + for (const [index, repository] of repositories.entries()) { + const state = pages.get(repository.fullName.toLowerCase()); + const page = Math.min(state?.page ?? 1, INBOX_PAGE_MAX); + pageState.writeUInt16BE(((state?.served ?? 0) << INBOX_PAGE_BITS) | page, index * 2); } + return createSignedToken({ + kind: INBOX_CURSOR_KIND, + identity, + ceiling, + window, + pages: pageState.toString('base64url'), + }); } +function decodeInboxCursor( + cursor: string | undefined, + identity: string, + cachedRepositories: BitbucketWorkspaceRepositoryAccess['repositories'] +): InboxCursorPosition | null { + if (!cursor) return null; + const payload = verifySignedToken(cursor, { + ttlSeconds: INBOX_CURSOR_TTL_SECONDS, + parse: value => { + const parsed = inboxCursorPayloadSchema.safeParse(value); + if (!parsed.success || parsed.data.identity !== identity) return null; + const repositories = inboxRepositoryWindow(cachedRepositories, parsed.data.window); + if (repositories.length === 0) return null; + const pageState = Buffer.from(parsed.data.pages, 'base64url'); + if (pageState.length !== repositories.length * 2) return null; + const pages = new Map(); + for (let index = 0; index < repositories.length; index += 1) { + const packed = pageState.readUInt16BE(index * 2); + const served = packed >> INBOX_PAGE_BITS; + if (served > BITBUCKET_PAGE_SIZE) return null; + pages.set(repositories[index].fullName.toLowerCase(), { + page: packed & INBOX_PAGE_MAX, + served, + }); + } + return { ceiling: parsed.data.ceiling, window: parsed.data.window, pages }; + }, + }); + return payload; +} /** * The ref of an inbox row: `workspace/repo-slug` from the destination * repository full name. Every published row names a repository in the diff --git a/apps/web/src/routers/provider-review-router.test.ts b/apps/web/src/routers/provider-review-router.test.ts index 9b5fb03b03..134c02da74 100644 --- a/apps/web/src/routers/provider-review-router.test.ts +++ b/apps/web/src/routers/provider-review-router.test.ts @@ -18,6 +18,7 @@ import { BITBUCKET_PR_REVIEW_CAPABILITIES, } from '@/lib/provider-review/bitbucket-write'; import { GITLAB_MR_REVIEW_CAPABILITIES } from '@/lib/provider-review/gitlab-write'; +import { createSignedToken } from '@/lib/signed-token'; import { providerLedgerResourceKey, providerReviewRouter } from './provider-review-router'; const ORG_ID = '2b1d4c8e-9f3a-4e5d-8c7b-6a5948372615'; @@ -277,6 +278,33 @@ describe('providerReviewRouter inputs', () => { undefined ); }); + it('round-trips a maximum-size signed inbox cursor through the router', async () => { + const base = { + kind: 'bitbucket-inbox-cursor', + identity: 'bitbucket-inbox:acme', + ceiling: '2026-09-02T00:00:00.000000+00:00', + served: [], + pages: [], + }; + let cursor = createSignedToken({ ...base, padding: '' }); + for (let length = 0; length < 2048; length += 1) { + const candidate = createSignedToken({ ...base, padding: 'x'.repeat(length) }); + if (candidate.length > 2048) break; + cursor = candidate; + } + expect(cursor.length).toBeLessThanOrEqual(2048); + bitbucketRead.listInbox.mockResolvedValueOnce({ items: [], nextCursor: cursor }); + await caller.listInbox({ platform: 'bitbucket', organizationId: ORG_ID }); + bitbucketRead.listInbox.mockResolvedValueOnce({ items: [], nextCursor: null }); + + await caller.listInbox({ platform: 'bitbucket', organizationId: ORG_ID, cursor }); + expect(cursor.length).toBeGreaterThan(2000); + + expect(bitbucketRead.listInbox).toHaveBeenLastCalledWith( + { type: 'organization', organizationId: ORG_ID, userId: USER_ID }, + cursor + ); + }); }); // ----- identity is server-derived ----------------------------------------------