diff --git a/apps/web/src/app/api/integrations/bitbucket/callback/route.test.ts b/apps/web/src/app/api/integrations/bitbucket/callback/route.test.ts index f91a430c45..62bc22faf1 100644 --- a/apps/web/src/app/api/integrations/bitbucket/callback/route.test.ts +++ b/apps/web/src/app/api/integrations/bitbucket/callback/route.test.ts @@ -16,11 +16,17 @@ import { import { scheduleBitbucketRepositoryCachePrime } from '@/lib/integrations/platforms/bitbucket/repository-cache'; import { getUserFromAuth } from '@/lib/user/server'; -jest.mock('@/lib/user/server'); +jest.mock('@/lib/constants', () => ({ APP_URL: 'http://localhost:3000' })); +jest.mock('@/lib/config.server', () => ({ NEXTAUTH_SECRET: 'callback-state-test-secret' })); +jest.mock('@/lib/user/server', () => ({ getUserFromAuth: jest.fn() })); +jest.mock('@/lib/organizations/trial-middleware', () => ({ + requireActiveSubscriptionOrTrial: jest.fn(), +})); jest.mock('@/routers/organizations/utils', () => ({ ensureOrganizationAccess: jest.fn(), })); jest.mock('@/lib/integrations/platforms/bitbucket/adapter', () => ({ + BitbucketOAuthScopeError: class extends Error {}, exchangeBitbucketOAuthCode: jest.fn(), fetchBitbucketUser: jest.fn(), fetchBitbucketWorkspaces: jest.fn(), @@ -28,6 +34,11 @@ jest.mock('@/lib/integrations/platforms/bitbucket/adapter', () => ({ jest.mock('@/lib/integrations/platforms/bitbucket/credentials', () => ({ BitbucketIntegrationAuthorizationError: class BitbucketIntegrationAuthorizationError extends Error {}, BitbucketIntegrationConnectionConflictError: class BitbucketIntegrationConnectionConflictError extends Error {}, + BitbucketIntegrationRecoveryError: class extends Error { + constructor(readonly code: string) { + super(code); + } + }, storeBitbucketIntegration: jest.fn(), })); jest.mock('@/lib/integrations/platforms/bitbucket/repository-cache', () => ({ @@ -208,3 +219,203 @@ describe('GET /api/integrations/bitbucket/callback', () => { expect(mockedCaptureException).not.toHaveBeenCalled(); }); }); + +const RECOVERY = { + integrationId: '33333333-3333-4333-8333-333333333333', + credentialId: '44444444-4444-4444-8444-444444444444', + credentialVersion: 2, + workspaceUuid: 'workspace-one', + workspaceSlug: 'workspace-one', +}; +const WRITE_TOKENS = { + ...BITBUCKET_TOKENS, + scopes: [...BITBUCKET_TOKENS.scopes, 'pullrequest:write'], +}; +const OLD_TOKENS = { ...BITBUCKET_TOKENS, accessToken: 'old-access', refreshToken: 'old-refresh' }; +let persistedTokens: BitbucketOAuthTokens; + +function recoveryState() { + return createOAuthState(`org_${ORGANIZATION_ID}`, USER_ID, undefined, RECOVERY); +} + +function expectRecoveryError(response: Response, code: string) { + expectRedirectLocation( + response, + `/organizations/${ORGANIZATION_ID}/integrations/bitbucket?error=${code}` + ); + expect(persistedTokens).toEqual(OLD_TOKENS); +} + +describe('Bitbucket OAuth recovery callback', () => { + beforeEach(() => { + jest.resetAllMocks(); + persistedTokens = OLD_TOKENS; + mockedGetUserFromAuth.mockResolvedValue({ + user: { id: USER_ID }, + authFailedResponse: null, + } as never); + mockedExchangeBitbucketOAuthCode.mockResolvedValue(WRITE_TOKENS); + mockedFetchBitbucketUser.mockResolvedValue(BITBUCKET_USER); + mockedFetchBitbucketWorkspaces.mockResolvedValue([WORKSPACE]); + mockedStoreBitbucketIntegration.mockImplementation(async input => { + // This boundary rejects ordinary first-connect, just like the existing stored connection. + if (!input.bitbucketRecovery) throw new BitbucketIntegrationConnectionConflictError(); + expect(input.owner).toEqual({ type: 'org', id: ORGANIZATION_ID }); + expect(input.authorizedByUserId).toBe(USER_ID); + expect(input.bitbucketRecovery).toEqual(RECOVERY); + persistedTokens = input.tokens; + return { status: 'connected', integrationId: RECOVERY.integrationId }; + }); + }); + + test('passes signed recovery through the real public callback without workspace selection', async () => { + mockedFetchBitbucketWorkspaces.mockResolvedValue([ + WORKSPACE, + { uuid: '{workspace-two}', slug: 'workspace-two', name: 'Two' }, + ]); + const response = await callPublicBitbucketCallback(makeRequest(recoveryState())); + expectRedirectLocation( + response, + `/organizations/${ORGANIZATION_ID}/integrations/bitbucket?success=connected` + ); + expect(persistedTokens).toEqual(WRITE_TOKENS); + }); + + test.each([ + ['access_denied', 'authorization_cancelled'], + ['server_error', 'connection_failed'], + ['invalid_scope', 'missing_scopes'], + ])('retains the existing connection after provider error %s', async (error, expectedError) => { + const request = makeRequest(recoveryState()); + request.nextUrl.searchParams.set('error', error); + expectRecoveryError(await callPublicBitbucketCallback(request), expectedError); + }); + + test('retains credentials when the provider returns no authorization code', async () => { + const request = makeRequest(recoveryState()); + request.nextUrl.searchParams.delete('code'); + expectRecoveryError(await callPublicBitbucketCallback(request), 'missing_code'); + }); + + test('rejects unsigned replacement selectors even with valid first-connect state', async () => { + const request = makeRequest(createOAuthState(`org_${ORGANIZATION_ID}`, USER_ID)); + request.nextUrl.searchParams.set('reconnectIntegrationId', RECOVERY.integrationId); + const response = await callPublicBitbucketCallback(request); + expectRecoveryError(response, 'invalid_state'); + }); + + test('preserves the legacy first-connect provider error redirect', async () => { + const request = makeRequest(createOAuthState(`org_${ORGANIZATION_ID}`, USER_ID)); + request.nextUrl.searchParams.set('error', 'server_error'); + expectRecoveryError(await callPublicBitbucketCallback(request), 'authorization_cancelled'); + }); + + test('retains the first-connect conflict when recovery is not signed', async () => { + expectRecoveryError( + await callPublicBitbucketCallback( + makeRequest(createOAuthState(`org_${ORGANIZATION_ID}`, USER_ID)) + ), + 'connection_exists' + ); + }); + + test('rejects a different callback actor before replacing credentials', async () => { + mockedGetUserFromAuth.mockResolvedValue({ + user: { id: 'oauth/different-user' }, + authFailedResponse: null, + } as never); + expectRecoveryError( + await callPublicBitbucketCallback(makeRequest(recoveryState())), + 'unauthorized' + ); + }); + + test('rechecks the organization role before replacing credentials', async () => { + const { ensureOrganizationAccess } = await import('@/routers/organizations/utils'); + jest.mocked(ensureOrganizationAccess).mockRejectedValue(new Error('management denied')); + expectRecoveryError( + await callPublicBitbucketCallback(makeRequest(recoveryState())), + 'unauthorized' + ); + }); + + test('retains credentials if manager authorization changes during persistence', async () => { + mockedStoreBitbucketIntegration.mockRejectedValue( + new BitbucketIntegrationAuthorizationError('authorization revoked') + ); + expectRecoveryError( + await callPublicBitbucketCallback(makeRequest(recoveryState())), + 'unauthorized' + ); + }); + + test('rejects tampered signed recovery without trusting its organization', async () => { + const state = recoveryState(); + const [encoded, signature] = state.split('.'); + const payload = JSON.parse(Buffer.from(encoded, 'base64url').toString('utf8')); + payload.bitbucketRecovery = { + ...RECOVERY, + integrationId: '55555555-5555-4555-8555-555555555555', + }; + const response = await callPublicBitbucketCallback( + makeRequest(`${Buffer.from(JSON.stringify(payload)).toString('base64url')}.${signature}`) + ); + expectRedirectLocation(response, '/integrations/bitbucket?error=invalid_state'); + expect(persistedTokens).toEqual(OLD_TOKENS); + }); + + test('does not replace read credentials with another read-only grant', async () => { + mockedExchangeBitbucketOAuthCode.mockResolvedValue(BITBUCKET_TOKENS); + expectRecoveryError( + await callPublicBitbucketCallback(makeRequest(recoveryState())), + 'missing_scopes' + ); + }); + + test('retains credentials when the account has no workspaces', async () => { + mockedFetchBitbucketWorkspaces.mockResolvedValue([]); + expectRecoveryError( + await callPublicBitbucketCallback(makeRequest(recoveryState())), + 'workspace_unavailable' + ); + }); + + test('explains missing base scopes from the exchange as a permission requirement', async () => { + const { BitbucketOAuthScopeError } = + await import('@/lib/integrations/platforms/bitbucket/adapter'); + mockedExchangeBitbucketOAuthCode.mockRejectedValue( + new BitbucketOAuthScopeError('scope_mismatch') + ); + expectRecoveryError( + await callPublicBitbucketCallback(makeRequest(recoveryState())), + 'missing_scopes' + ); + }); + + test.each(['connection_changed', 'workspace_unavailable', 'missing_scopes'] as const)( + 'reports a non-destructive persistence rejection: %s', + async code => { + const { BitbucketIntegrationRecoveryError } = + await import('@/lib/integrations/platforms/bitbucket/credentials'); + mockedStoreBitbucketIntegration.mockRejectedValue( + new BitbucketIntegrationRecoveryError(code) + ); + expectRecoveryError(await callPublicBitbucketCallback(makeRequest(recoveryState())), code); + } + ); + + test.each(['exchange', 'profile', 'workspaces', 'persistence'])( + 'retains credentials after a retryable %s failure', + async phase => { + const failure = new Error('Temporary provider or storage failure'); + if (phase === 'exchange') mockedExchangeBitbucketOAuthCode.mockRejectedValue(failure); + if (phase === 'profile') mockedFetchBitbucketUser.mockRejectedValue(failure); + if (phase === 'workspaces') mockedFetchBitbucketWorkspaces.mockRejectedValue(failure); + if (phase === 'persistence') mockedStoreBitbucketIntegration.mockRejectedValue(failure); + expectRecoveryError( + await callPublicBitbucketCallback(makeRequest(recoveryState())), + 'connection_failed' + ); + } + ); +}); diff --git a/apps/web/src/app/api/integrations/bitbucket/connect/route.test.ts b/apps/web/src/app/api/integrations/bitbucket/connect/route.test.ts index 810bb791ef..5e27f3f71c 100644 --- a/apps/web/src/app/api/integrations/bitbucket/connect/route.test.ts +++ b/apps/web/src/app/api/integrations/bitbucket/connect/route.test.ts @@ -1,62 +1,98 @@ import { beforeEach, describe, expect, test } from '@jest/globals'; import { ORGANIZATION_BILLING_ROLES } from '@kilocode/app-shared/organizations'; +import { TRPCError } from '@trpc/server'; import { NextRequest } from 'next/server'; import { getUserFromAuth } from '@/lib/user/server'; import { verifyOAuthState } from '@/lib/integrations/oauth-state'; +import { + BitbucketIntegrationRecoveryError, + getBitbucketOAuthRecovery, +} from '@/lib/integrations/platforms/bitbucket/credentials'; import { ensureOrganizationAccess } from '@/routers/organizations/utils'; +jest.mock('@/lib/constants', () => ({ APP_URL: 'http://localhost:3000' })); jest.mock('@/lib/config.server', () => ({ BITBUCKET_CLIENT_ID: 'bitbucket-client-id', NEXTAUTH_SECRET: 'test-nextauth-secret', })); -jest.mock('@/lib/user/server'); +jest.mock('@/lib/user/server', () => ({ getUserFromAuth: jest.fn() })); +jest.mock('@/lib/organizations/trial-middleware', () => ({ + requireActiveSubscriptionOrTrial: jest.fn(), +})); jest.mock('@/routers/organizations/utils', () => ({ ensureOrganizationAccess: jest.fn(), })); +jest.mock('@/lib/integrations/platforms/bitbucket/credentials', () => ({ + getBitbucketOAuthRecovery: jest.fn(), + BitbucketIntegrationRecoveryError: class extends Error { + constructor(readonly code: string) { + super(code); + } + }, +})); jest.mock('@sentry/nextjs', () => ({ captureException: jest.fn(), })); const mockedGetUserFromAuth = jest.mocked(getUserFromAuth); const mockedEnsureOrganizationAccess = jest.mocked(ensureOrganizationAccess); +const mockedGetRecovery = jest.mocked(getBitbucketOAuthRecovery); const USER_ID = '034489e8-19e0-4479-9d69-2edad719e847'; const ORGANIZATION_ID = '7e3011af-e99d-444f-8171-54c2225b87dc'; +const RECOVERY = { + integrationId: '33333333-3333-4333-8333-333333333333', + credentialId: '44444444-4444-4444-8444-444444444444', + credentialVersion: 2, + workspaceUuid: 'workspace-one', + workspaceSlug: 'workspace-one', +}; -async function callPublicBitbucketConnect() { +async function callPublicBitbucketConnect(query: Record = {}) { const { GET } = await import('../../[platform]/connect/route'); + const search = new URLSearchParams({ + organizationId: ORGANIZATION_ID, + returnTo: `/organizations/${ORGANIZATION_ID}/integrations/bitbucket`, + ...query, + }); return GET( - new NextRequest( - `http://localhost:3000/api/integrations/bitbucket/connect?organizationId=${ORGANIZATION_ID}&returnTo=%2Forganizations%2F${ORGANIZATION_ID}%2Fintegrations%2Fbitbucket` - ), - { - params: Promise.resolve({ platform: 'bitbucket' }), - } + new NextRequest(`http://localhost:3000/api/integrations/bitbucket/connect?${search}`), + { params: Promise.resolve({ platform: 'bitbucket' }) } ); } +function redirectUrl(response: Response) { + expect(response.status).toBe(307); + return new URL(response.headers.get('location') ?? ''); +} + describe('GET /api/integrations/bitbucket/connect', () => { beforeEach(() => { - jest.clearAllMocks(); + jest.resetAllMocks(); mockedGetUserFromAuth.mockResolvedValue({ user: { id: USER_ID }, authFailedResponse: null, } as never); mockedEnsureOrganizationAccess.mockResolvedValue('owner'); + mockedGetRecovery.mockImplementation(async (owner, integrationId) => { + if ( + owner.type !== 'org' || + owner.id !== ORGANIZATION_ID || + integrationId !== RECOVERY.integrationId + ) { + throw new BitbucketIntegrationRecoveryError('connection_changed'); + } + return RECOVERY; + }); }); - test('dispatches the public OAuth route with organization ownership and required scopes', async () => { - const response = await callPublicBitbucketConnect(); - - expect(response.status).toBe(307); - const location = response.headers.get('location'); - expect(location).toBeTruthy(); - const url = new URL(location ?? ''); + test('dispatches normal first-connect without recovery and requests the added scope', async () => { + const url = redirectUrl(await callPublicBitbucketConnect()); expect(`${url.origin}${url.pathname}`).toBe('https://bitbucket.org/site/oauth2/authorize'); expect(Object.fromEntries(url.searchParams)).toEqual( expect.objectContaining({ client_id: 'bitbucket-client-id', response_type: 'code', - scope: 'account repository:write pullrequest webhook', + scope: 'account repository:write pullrequest webhook pullrequest:write', }) ); expect(verifyOAuthState(url.searchParams.get('state'))).toEqual({ @@ -70,4 +106,74 @@ describe('GET /api/integrations/bitbucket/connect', () => { ORGANIZATION_BILLING_ROLES ); }); + + test('signs the current authorized recovery target rather than unsigned workspace selectors', async () => { + const url = redirectUrl( + await callPublicBitbucketConnect({ + reconnectIntegrationId: RECOVERY.integrationId, + workspaceUuid: 'attacker-workspace', + workspaceSlug: 'attacker-slug', + }) + ); + expect(url.origin).toBe('https://bitbucket.org'); + expect(verifyOAuthState(url.searchParams.get('state'))).toEqual({ + owner: `org_${ORGANIZATION_ID}`, + userId: USER_ID, + returnTo: `/organizations/${ORGANIZATION_ID}/integrations/bitbucket`, + bitbucketRecovery: RECOVERY, + }); + }); + + test('preserves the legacy first-connect authorization error', async () => { + mockedEnsureOrganizationAccess.mockRejectedValue(new TRPCError({ code: 'UNAUTHORIZED' })); + const url = redirectUrl(await callPublicBitbucketConnect()); + expect(url.searchParams.get('error')).toBe('oauth_init_failed'); + expect(url.searchParams.has('state')).toBe(false); + }); + + test('denies a non-manager before issuing recovery state', async () => { + mockedEnsureOrganizationAccess.mockRejectedValue(new TRPCError({ code: 'UNAUTHORIZED' })); + const url = redirectUrl( + await callPublicBitbucketConnect({ reconnectIntegrationId: RECOVERY.integrationId }) + ); + expect(`${url.pathname}${url.search}`).toBe( + `/organizations/${ORGANIZATION_ID}/integrations/bitbucket?error=unauthorized` + ); + expect(url.searchParams.has('state')).toBe(false); + }); + + test.each(['', 'not-a-uuid', '55555555-5555-4555-8555-555555555555'])( + 'rejects an absent or mismatched recovery integration: %s', + async reconnectIntegrationId => { + const url = redirectUrl(await callPublicBitbucketConnect({ reconnectIntegrationId })); + expect(url.searchParams.get('error')).toBe('connection_changed'); + expect(url.searchParams.has('state')).toBe(false); + } + ); + + test('rejects recovery after disconnection', async () => { + mockedGetRecovery.mockRejectedValue( + new BitbucketIntegrationRecoveryError('connection_changed') + ); + const url = redirectUrl( + await callPublicBitbucketConnect({ reconnectIntegrationId: RECOVERY.integrationId }) + ); + expect(url.searchParams.get('error')).toBe('connection_changed'); + expect(url.searchParams.has('state')).toBe(false); + }); + + test('requires sign-in without starting recovery', async () => { + mockedGetUserFromAuth.mockResolvedValue({ + user: null, + authFailedResponse: new Response(null, { status: 401 }), + } as never); + const url = redirectUrl( + await callPublicBitbucketConnect({ reconnectIntegrationId: RECOVERY.integrationId }) + ); + expect(url.pathname).toBe('/users/sign_in'); + expect(url.searchParams.get('callbackPath')).toBe( + `/organizations/${ORGANIZATION_ID}/integrations/bitbucket` + ); + expect(url.searchParams.has('state')).toBe(false); + }); }); diff --git a/apps/web/src/components/integrations/BitbucketConnectSetup.test.ts b/apps/web/src/components/integrations/BitbucketConnectSetup.test.ts new file mode 100644 index 0000000000..69f5583135 --- /dev/null +++ b/apps/web/src/components/integrations/BitbucketConnectSetup.test.ts @@ -0,0 +1,98 @@ +import React, { createElement } from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { getUnexpectedBitbucketWorkspaceAccessTokenScopes } from '@kilocode/worker-utils/bitbucket-workspace-access-token'; +import { + BitbucketConnectSetup, + buildConnectedWorkspaceAccessTokenStatus, +} from './BitbucketConnectSetup'; + +jest.mock('@/lib/trpc/utils', () => ({ + useTRPC: () => ({ + organizations: { + bitbucket: { + getStatus: { queryKey: () => ['status'] }, + connect: { mutationOptions: (options: object) => options }, + }, + cloudAgentNext: { listBitbucketRepositories: { queryKey: () => ['repositories'] } }, + reviewAgent: { getBitbucketReadiness: { queryKey: () => ['readiness'] } }, + }, + }), +})); + +// Match the repository's classic JSX Jest transform without replacing the shipped components. +Object.assign(globalThis, { React }); + +function render(canManage = true, statusRefetchFailed = false) { + return renderToStaticMarkup( + createElement( + QueryClientProvider, + { client: new QueryClient() }, + createElement(BitbucketConnectSetup, { + organizationId: 'organization-1', + canManage, + statusRefetchFailed, + }) + ) + ); +} + +it('AC1 keeps the exact workspace connected after replacing the token with write grants', () => { + const status = buildConnectedWorkspaceAccessTokenStatus( + { + integrationId: 'integration-1', + workspace: { uuid: '{workspace-1}', slug: 'acme', displayName: 'Acme Workspace' }, + credentialVersion: 2, + repositoryCount: 1, + validatedAt: '2026-08-30T09:00:00.000Z', + unexpectedScopes: getUnexpectedBitbucketWorkspaceAccessTokenScopes([ + 'account', + 'pullrequest:write', + 'webhook', + ]), + }, + true + ); + expect(status).toMatchObject({ + status: 'connected', + method: 'workspace_access_token', + integrationId: 'integration-1', + workspace: { uuid: '{workspace-1}', slug: 'acme', displayName: 'Acme Workspace' }, + lastValidatedAt: '2026-08-30T09:00:00.000Z', + unexpectedScopes: [], + recoveryAction: null, + canManage: true, + repositoryCache: { status: 'uninitialized', repositories: [], syncedAt: null }, + }); +}); + +it('AC1 explains write recovery while keeping the empty connection form usable', () => { + const html = render(); + expect(html).toContain('
  • Pull request Write
  • '); + expect(html).toContain('Existing Pull request Read connections remain readable'); + expect(html).toContain('replace the workspace token'); + expect(html).toContain('reconnect with OAuth'); + expect(html).toContain('the Kilo operator must enable Pull request Write'); + expect(html).toContain('Bitbucket OAuth consumer'); + expect(html).toContain('configuration requirement, not a Bitbucket review limitation'); + expect(html).toContain('for="bitbucket-workspace-token"'); + expect(html).toContain('id="bitbucket-review-permissions"'); + expect(html).toContain( + 'aria-describedby="bitbucket-workspace-token-help bitbucket-review-permissions"' + ); + expect(html).toMatch(/]*disabled=""[^>]*>Connect workspace<\/button>/); +}); + +it('AC1 retains the connection form after a retryable status refresh failure', () => { + const html = render(true, true); + expect(html).toContain('Bitbucket status could not be refreshed'); + expect(html).toContain('Showing the last loaded integration status'); + expect(html).toContain('id="bitbucket-workspace-token"'); +}); + +it('AC1 explains management denial without an unauthorized connection action', () => { + const html = render(false); + expect(html).toContain('An organization owner or billing manager can connect'); + expect(html).not.toContain('id="bitbucket-workspace-token"'); + expect(html).not.toContain('Connect with Bitbucket OAuth'); +}); diff --git a/apps/web/src/components/integrations/BitbucketConnectSetup.tsx b/apps/web/src/components/integrations/BitbucketConnectSetup.tsx index c55406603a..fe4e4e90fc 100644 --- a/apps/web/src/components/integrations/BitbucketConnectSetup.tsx +++ b/apps/web/src/components/integrations/BitbucketConnectSetup.tsx @@ -32,7 +32,7 @@ const REQUIRED_PERMISSIONS = [ 'Account Read', 'Repository Read', 'Repository Write', - 'Pull request Read', + 'Pull request Write', 'Webhooks Read and Write', ]; @@ -92,6 +92,35 @@ function CardHeaderContent() { ); } +export function BitbucketReviewPermissions() { + return ( +
    +

    + Existing Pull request Read connections remain readable. To enable approvals and merges, + replace the workspace token with Pull request Write, or reconnect with OAuth. +

    +

    + If OAuth rejects the added scope, the Kilo operator must enable Pull request Write on the + Bitbucket OAuth consumer. This is a configuration requirement, not a Bitbucket review + limitation. +

    +
    + ); +} + +export function BitbucketTokenPermissions({ id }: { id: string }) { + return ( +
    +

    Required permissions

    +
      + {REQUIRED_PERMISSIONS.map(permission => ( +
    • {permission}
    • + ))} +
    +
    + ); +} + export function BitbucketConnectSetup({ organizationId, canManage, @@ -206,6 +235,8 @@ export function BitbucketConnectSetup({ + + -
    -

    Required permissions

    -
      - {REQUIRED_PERMISSIONS.map(permission => ( -
    • {permission}
    • - ))} -
    -
    +
    @@ -268,7 +292,7 @@ export function BitbucketConnectSetup({ required maxLength={8192} className="h-control-touch sm:h-9" - aria-describedby="bitbucket-workspace-token-help" + aria-describedby="bitbucket-workspace-token-help bitbucket-review-permissions" />

    )} + {status.status === 'workspace_selection_required' ? ( ) : ( diff --git a/apps/web/src/components/integrations/BitbucketIntegrationControls.tsx b/apps/web/src/components/integrations/BitbucketIntegrationControls.tsx index 9d651edb40..f6800423f8 100644 --- a/apps/web/src/components/integrations/BitbucketIntegrationControls.tsx +++ b/apps/web/src/components/integrations/BitbucketIntegrationControls.tsx @@ -32,7 +32,10 @@ import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; import { SecretTokenInput } from '@/components/ui/secret-token-input'; import { useTRPC } from '@/lib/trpc/utils'; -import { buildConnectedWorkspaceAccessTokenStatus } from './BitbucketConnectSetup'; +import { + BitbucketTokenPermissions, + buildConnectedWorkspaceAccessTokenStatus, +} from './BitbucketConnectSetup'; const REVOKE_WORKSPACE_TOKEN_URL = 'https://support.atlassian.com/bitbucket-cloud/docs/revoke-a-workspace-access-token/'; @@ -149,6 +152,7 @@ function ReplaceTokenDialog({ )} mutation.isPending && event.preventDefault()} onPointerDownOutside={event => mutation.isPending && event.preventDefault()} @@ -178,6 +182,7 @@ function ReplaceTokenDialog({

    +

    Kilo encrypts this token. It is never shown again after submission. @@ -374,6 +379,20 @@ export function BitbucketIntegrationControls({ )}

    + {status.method === 'oauth' && status.status === 'connected' && ( + + )} {status.method === 'workspace_access_token' && ( ({ + useTRPC: () => ({ + organizations: { + bitbucket: { + getStatus: { + queryKey: () => ['status'], + queryOptions: () => ({ queryKey: ['status'], queryFn: skipToken }), + }, + connect: { mutationOptions: (options: object) => options }, + replaceToken: { mutationOptions: (options: object) => options }, + refreshRepositories: { mutationOptions: (options: object) => options }, + disconnect: { mutationOptions: () => ({}) }, + }, + cloudAgentNext: { listBitbucketRepositories: { queryKey: () => ['repositories'] } }, + reviewAgent: { getBitbucketReadiness: { queryKey: () => ['readiness'] } }, + }, + }), +})); + +let mockOpenDialogs = false; + +// Render the real dialog content inline for the Node renderer; E13 checks live interaction. +jest.mock('@radix-ui/react-dialog', () => { + const actual = jest.requireActual('@radix-ui/react-dialog'); + return { + ...actual, + Root: (props: ComponentProps) => + createElement(actual.Root, { ...props, open: mockOpenDialogs || props.open }), + Portal: ({ children }: { children: ReactNode }) => children, + }; +}); + +// Match the repository's classic JSX Jest transform without replacing the shipped components. +Object.assign(globalThis, { React }); + +type BitbucketStatus = ReturnType; + +function connectedStatus() { + return { + status: 'connected', + recoveryAction: null, + method: 'workspace_access_token', + integrationId: '33333333-3333-4333-8333-333333333333', + integrationStatus: 'active', + workspace: { + uuid: '11111111-1111-4111-8111-111111111111', + slug: 'acme', + displayName: 'Acme Workspace', + }, + invalidatedAt: null, + invalidationReason: null, + lastValidatedAt: '2026-08-30T09:00:00.000Z', + unexpectedScopes: [], + repositoryCache: { + status: 'available', + syncedAt: '2026-08-30T09:00:00.000Z', + repositories: [ + { + id: '22222222-2222-4222-8222-222222222222', + workspaceUuid: '11111111-1111-4111-8111-111111111111', + name: 'mobile', + fullName: 'acme/mobile', + private: true, + defaultBranch: 'main', + }, + ], + }, + canManage: true, + } satisfies BitbucketStatus; +} + +function renderDetails( + status: BitbucketStatus, + { + openDialogs = false, + refetchFailed = false, + error, + }: { openDialogs?: boolean; refetchFailed?: boolean; error?: string } = {} +) { + const client = new QueryClient(); + client.setQueryData(['status'], status); + if (refetchFailed) { + client + .getQueryCache() + .find({ queryKey: ['status'] }) + ?.setState({ + status: 'error', + error: new Error('Temporary status failure'), + }); + } + mockOpenDialogs = openDialogs; + try { + return renderToStaticMarkup( + createElement( + QueryClientProvider, + { client }, + createElement(BitbucketIntegrationDetails, { organizationId: 'organization-1', error }) + ) + ); + } finally { + mockOpenDialogs = false; + client.clear(); + } +} + describe('Bitbucket integration UI state', () => { + it('AC1 exposes write recovery from the connected parent without hiding repositories', () => { + const html = renderDetails(connectedStatus()); + expect(html).toContain('Existing Pull request Read connections remain readable'); + expect(html).toContain('replace the workspace token with Pull request Write'); + expect(html).toContain('Replace token'); + expect(html).toContain('Acme Workspace'); + expect(html).toContain('acme/mobile'); + expect(html).toContain('Refresh repositories'); + expect(html).not.toContain('Not connected'); + expect(html).not.toContain('id="bitbucket-workspace-token"'); + expect(html).not.toContain('id="bitbucket-replacement-token"'); + }); + + it('AC11 links required permissions to the replacement input through the connected parent', () => { + const html = renderDetails(connectedStatus(), { openDialogs: true }); + const form = html.match(/]*>[\s\S]*?<\/form>/)?.[0]; + expect(form).toBeDefined(); + const dialog = html.match(/]*role="dialog"[^>]*>/)?.[0]; + expect(dialog).toContain('max-h-[calc(100dvh-2rem)]'); + expect(dialog).toContain('overflow-y-auto'); + expect(form).toContain('Replace Workspace Access Token'); + for (const permission of [ + 'Account Read', + 'Repository Read', + 'Repository Write', + 'Pull request Write', + 'Webhooks Read and Write', + ]) { + expect(form).toContain(`
  • ${permission}
  • `); + } + expect(form).toContain('id="bitbucket-replacement-permissions"'); + expect(form).toMatch( + /]*id="bitbucket-replacement-token"[^>]*aria-describedby="[^"]*bitbucket-replacement-permissions/ + ); + expect(form).toContain('Keep current token'); + expect(form).toMatch(/]*type="submit"[^>]*disabled=""[^>]*>Replace token<\/button>/); + expect(html).toContain('acme/mobile'); + }); + + it('AC1 keeps replacement guidance and cached repositories after a retryable status failure', () => { + const html = renderDetails(connectedStatus(), { openDialogs: true, refetchFailed: true }); + expect(html).toContain('Bitbucket status could not be refreshed'); + expect(html).toContain('Showing the last loaded workspace and repository cache'); + expect(html).toContain('acme/mobile'); + expect(html).toContain('id="bitbucket-replacement-token"'); + expect(html).toContain('
  • Pull request Write
  • '); + expect(html).toContain('Keep current token'); + expect(html).toContain('Replace token'); + }); + + it('AC1 explains write permission recovery to non-managers without management controls', () => { + const html = renderDetails({ ...connectedStatus(), canManage: false }, { openDialogs: true }); + expect(html).toContain('Pull request Write'); + expect(html).toContain('An organization owner or billing manager'); + expect(html).toContain('acme/mobile'); + expect(html).not.toContain('id="bitbucket-replacement-token"'); + expect(html).not.toContain('Replace token'); + expect(html).not.toContain('Disconnect Bitbucket'); + }); + + it('AC1 exposes an actionable OAuth recovery link from the connected parent', () => { + const status = { + ...connectedStatus(), + method: 'oauth', + authorizingNickname: 'bucket-user', + } satisfies BitbucketStatus; + const html = renderDetails(status); + const href = html.match(/]*href="([^"]+)"[^>]*>Reconnect with OAuth<\/a>/)?.[1]; + expect(href).toBeDefined(); + const url = new URL(href?.replaceAll('&', '&') ?? '', 'https://app.example'); + expect(url.pathname).toBe('/api/integrations/bitbucket/connect'); + expect(url.searchParams.get('organizationId')).toBe('organization-1'); + expect(url.searchParams.get('reconnectIntegrationId')).toBe(status.integrationId); + expect(html).toContain('the Kilo operator must enable Pull request Write'); + expect(html).toContain('Bitbucket OAuth consumer'); + expect(html).toContain('bucket-user'); + expect(html).toContain('acme/mobile'); + expect(html).not.toContain('Replace token'); + expect(html).not.toContain('Choose workspace'); + }); + + it.each(['authorization_cancelled', 'connection_failed', 'oauth_init_failed'])( + 'AC1 retains OAuth recovery and cached repositories after %s', + error => { + const html = renderDetails( + { ...connectedStatus(), method: 'oauth', authorizingNickname: 'bucket-user' }, + { error } + ); + expect(html).toContain(getBitbucketConnectionErrorMessage(error)); + expect(html).toContain('>Reconnect with OAuth'); + expect(html).toContain('acme/mobile'); + expect(html).toContain('Acme Workspace'); + expect(html).toContain('Refresh repositories'); + expect(html).not.toContain('Not connected'); + expect(html).not.toContain('Choose workspace'); + } + ); + + it.each([ + ['connection_changed', 'Refresh this page'], + ['workspace_unavailable', 'account with access to that workspace'], + ['missing_scopes', 'Kilo operator'], + ])('AC1 explains %s without replacing the connected presentation', (error, guidance) => { + const html = renderDetails( + { ...connectedStatus(), method: 'oauth', authorizingNickname: 'bucket-user' }, + { error } + ); + expect(html).toContain(guidance); + expect(html).toContain('acme/mobile'); + expect(html).toContain('>Reconnect with OAuth'); + expect(html).not.toContain('Not connected'); + }); + + it('AC1 denies OAuth recovery controls to non-managers while retaining repositories', () => { + const html = renderDetails( + { + ...connectedStatus(), + method: 'oauth', + authorizingNickname: 'bucket-user', + canManage: false, + }, + { error: 'unauthorized' } + ); + expect(html).toContain('Ask an owner or billing manager'); + expect(html).toContain('acme/mobile'); + expect(html).not.toContain('>Reconnect with OAuth'); + expect(html).not.toContain('reconnectIntegrationId='); + expect(html).not.toContain('Disconnect Bitbucket'); + }); + + it('AC1 keeps the actual disconnected parent on the existing first-connect form', () => { + const html = renderDetails({ + ...connectedStatus(), + status: 'not_connected', + integrationId: null, + integrationStatus: null, + workspace: null, + lastValidatedAt: null, + repositoryCache: { status: 'uninitialized', repositories: [], syncedAt: null }, + }); + expect(html).toContain('Not connected'); + expect(html).toContain('id="bitbucket-workspace-token"'); + expect(html).toContain('>Connect workspace'); + expect(html).not.toContain('reconnectIntegrationId='); + expect(html).not.toContain('Integration controls'); + }); + it('builds connected status from a successful Workspace Access Token mutation', () => { expect( buildConnectedWorkspaceAccessTokenStatus( diff --git a/apps/web/src/components/integrations/BitbucketIntegrationDetails.tsx b/apps/web/src/components/integrations/BitbucketIntegrationDetails.tsx index 22af14d23a..9f5e1bab5b 100644 --- a/apps/web/src/components/integrations/BitbucketIntegrationDetails.tsx +++ b/apps/web/src/components/integrations/BitbucketIntegrationDetails.tsx @@ -28,6 +28,12 @@ const bitbucketConnectionErrorMessages: Record = { connection_exists: 'Bitbucket is already connected. Disconnect the current connection before using OAuth.', connection_failed: 'Bitbucket could not be connected. Try OAuth again in a minute.', + connection_changed: + 'The Bitbucket connection changed. No credentials were replaced. Refresh this page before reconnecting with OAuth.', + workspace_unavailable: + 'The authorized Bitbucket account cannot access the connected workspace. Use an account with access to that workspace. The current connection is unchanged.', + missing_scopes: + 'Bitbucket did not grant the required permissions. Ask the Kilo operator to enable Account Read, Repository Write, Pull request Write, and Webhooks on the OAuth consumer, then reconnect. The current connection is unchanged.', }; export function getBitbucketConnectionErrorMessage(error: string): string { diff --git a/apps/web/src/lib/integrations/oauth-state.test.ts b/apps/web/src/lib/integrations/oauth-state.test.ts index effac917ea..16d777001c 100644 --- a/apps/web/src/lib/integrations/oauth-state.test.ts +++ b/apps/web/src/lib/integrations/oauth-state.test.ts @@ -1,6 +1,99 @@ -import { createOAuthState, verifyOAuthState } from './oauth-state'; +import { createHmac } from 'node:crypto'; +import { createOAuthState, OAUTH_STATE_TTL_SECONDS, verifyOAuthState } from './oauth-state'; + +jest.mock('@/lib/config.server', () => ({ NEXTAUTH_SECRET: 'oauth-state-test-secret' })); + +const recovery = { + integrationId: '33333333-3333-4333-8333-333333333333', + credentialId: '44444444-4444-4444-8444-444444444444', + credentialVersion: 2, + workspaceUuid: 'workspace-one', + workspaceSlug: 'workspace-one', +}; + +function signPayload(payload: object) { + const encoded = Buffer.from(JSON.stringify(payload)).toString('base64url'); + return `${encoded}.${createHmac('sha256', 'oauth-state-test-secret').update(encoded).digest('base64url')}`; +} describe('oauth state', () => { + test('binds Bitbucket recovery to the owner, actor, credential revision, and workspace', () => { + const state = createOAuthState( + 'org_organization', + 'oauth/actor', + '/integrations/bitbucket', + recovery + ); + expect(verifyOAuthState(state)).toEqual({ + owner: 'org_organization', + userId: 'oauth/actor', + returnTo: '/integrations/bitbucket', + bitbucketRecovery: recovery, + }); + }); + + test.each([ + ['integrationId', '55555555-5555-4555-8555-555555555555'], + ['credentialId', '66666666-6666-4666-8666-666666666666'], + ['credentialVersion', 3], + ['workspaceUuid', 'workspace-two'], + ['workspaceSlug', 'workspace-two'], + ] as const)('rejects a tampered recovery %s', (field, value) => { + const state = createOAuthState('org_organization', 'oauth/actor', undefined, recovery); + const [encoded, signature] = state.split('.'); + const payload = JSON.parse(Buffer.from(encoded, 'base64url').toString('utf8')); + payload.bitbucketRecovery = { ...recovery, [field]: value }; + const tampered = Buffer.from(JSON.stringify(payload)).toString('base64url'); + expect(verifyOAuthState(`${tampered}.${signature}`)).toBeNull(); + }); + + test.each([ + null, + {}, + { ...recovery, integrationId: '' }, + { ...recovery, credentialId: 'not-a-uuid' }, + { ...recovery, credentialVersion: 0 }, + { ...recovery, credentialVersion: 1.5 }, + { ...recovery, credentialVersion: 2_147_483_647 }, + { ...recovery, workspaceUuid: '' }, + { ...recovery, workspaceSlug: ' padded' }, + { ...recovery, replaceAnything: true }, + ])( + 'rejects malformed signed recovery instead of downgrading to first-connect', + bitbucketRecovery => { + expect( + verifyOAuthState( + signPayload({ + owner: 'org_organization', + uid: 'oauth/actor', + iat: Math.floor(Date.now() / 1000), + nonce: 'nonce', + bitbucketRecovery, + }) + ) + ).toBeNull(); + } + ); + + test('expires recovery with the existing state lifetime', () => { + const now = Date.now(); + const clock = jest.spyOn(Date, 'now').mockReturnValue(now); + try { + const state = createOAuthState('org_organization', 'oauth/actor', undefined, recovery); + clock.mockReturnValue(now + (OAUTH_STATE_TTL_SECONDS + 1) * 1000); + expect(verifyOAuthState(state)).toBeNull(); + } finally { + clock.mockRestore(); + } + }); + + test('preserves the legacy two-argument state contract for other providers', () => { + expect(verifyOAuthState(createOAuthState('google:encoded-owner', 'oauth/actor'))).toEqual({ + owner: 'google:encoded-owner', + userId: 'oauth/actor', + }); + }); + test('round-trips a validated return path', () => { const state = createOAuthState('user_123', 'user_123', '/claw/new?step=linear'); diff --git a/apps/web/src/lib/integrations/oauth-state.ts b/apps/web/src/lib/integrations/oauth-state.ts index 387782790f..4b4083b465 100644 --- a/apps/web/src/lib/integrations/oauth-state.ts +++ b/apps/web/src/lib/integrations/oauth-state.ts @@ -2,6 +2,25 @@ import 'server-only'; import crypto from 'node:crypto'; import { NEXTAUTH_SECRET } from '@/lib/config.server'; import { validateReturnPath } from '@/lib/integrations/validate-return-path'; +import { z } from 'zod'; + +export const BitbucketOAuthRecoverySchema = z + .object({ + integrationId: z.uuid(), + credentialId: z.uuid(), + credentialVersion: z.number().int().positive().max(2_147_483_646), + workspaceUuid: z + .string() + .min(1) + .refine(value => value.trim() === value), + workspaceSlug: z + .string() + .min(1) + .refine(value => value.trim() === value), + }) + .strict(); + +export type BitbucketOAuthRecovery = z.infer; /** * HMAC-signed OAuth state parameter. @@ -13,7 +32,7 @@ import { validateReturnPath } from '@/lib/integrations/validate-return-path'; * * This module produces a state value of the form: * - * base64url({ owner, uid, iat, nonce }) . HMAC-SHA256(payload, secret) + * base64url({ owner, uid, iat, nonce, ...optionalContext }) . HMAC-SHA256(payload, secret) * * where `owner` is the original owner string, `uid` is the ID of the * authenticated user who started the flow, `iat` is the issued-at timestamp @@ -25,8 +44,7 @@ import { validateReturnPath } from '@/lib/integrations/validate-return-path'; * 2. Check `iat` is within the allowed TTL window (default 10 minutes). * 3. Extract `uid` and confirm it matches the session user (same user * who initiated the flow is completing it). - * 4. Return the `owner` string so the rest of the callback logic is - * unchanged. + * 4. Return the owner and any validated, signed recovery context. */ const HMAC_ALGORITHM = 'sha256'; @@ -47,7 +65,12 @@ function sign(data: string): string { * @param owner – owner string, e.g. `user_abc123` or `org_xyz789` * @param userId – the ID of the currently-authenticated user initiating the flow */ -export function createOAuthState(owner: string, userId: string, returnTo?: string): string { +export function createOAuthState( + owner: string, + userId: string, + returnTo?: string, + bitbucketRecovery?: BitbucketOAuthRecovery +): string { const iat = Math.floor(Date.now() / 1000); const nonce = crypto.randomBytes(NONCE_BYTES).toString('base64url'); const safeReturnTo = returnTo ? validateReturnPath(returnTo) : null; @@ -58,6 +81,9 @@ export function createOAuthState(owner: string, userId: string, returnTo?: strin iat, nonce, ...(safeReturnTo ? { returnTo: safeReturnTo } : {}), + ...(bitbucketRecovery + ? { bitbucketRecovery: BitbucketOAuthRecoverySchema.parse(bitbucketRecovery) } + : {}), }) ).toString('base64url'); const signature = sign(payload); @@ -71,6 +97,8 @@ export type VerifiedOAuthState = { userId: string; /** Optional relative path to return to after the OAuth callback. */ returnTo?: string; + /** Explicit Bitbucket recovery; absent on legacy first-connect state. */ + bitbucketRecovery?: BitbucketOAuthRecovery; }; /** @@ -106,6 +134,7 @@ export function verifyOAuthState(state: string | null): VerifiedOAuthState | nul iat?: number; nonce?: string; returnTo?: string; + bitbucketRecovery?: unknown; }; if (typeof data.owner !== 'string' || typeof data.uid !== 'string') return null; @@ -118,8 +147,18 @@ export function verifyOAuthState(state: string | null): VerifiedOAuthState | nul if (typeof data.nonce !== 'string' || data.nonce.length === 0) return null; const returnTo = typeof data.returnTo === 'string' ? validateReturnPath(data.returnTo) : null; - - return { owner: data.owner, userId: data.uid, ...(returnTo ? { returnTo } : {}) }; + const recovery = + data.bitbucketRecovery === undefined + ? undefined + : BitbucketOAuthRecoverySchema.safeParse(data.bitbucketRecovery); + if (recovery && !recovery.success) return null; + + return { + owner: data.owner, + userId: data.uid, + ...(returnTo ? { returnTo } : {}), + ...(recovery?.success ? { bitbucketRecovery: recovery.data } : {}), + }; } catch { return null; } diff --git a/apps/web/src/lib/integrations/oauth/platforms/bitbucket-callback.ts b/apps/web/src/lib/integrations/oauth/platforms/bitbucket-callback.ts index 22a8f4fc53..1783c628b9 100644 --- a/apps/web/src/lib/integrations/oauth/platforms/bitbucket-callback.ts +++ b/apps/web/src/lib/integrations/oauth/platforms/bitbucket-callback.ts @@ -9,7 +9,9 @@ import { parseOAuthStateOwner, } from '@/lib/integrations/oauth/common'; import { verifyOAuthState } from '@/lib/integrations/oauth-state'; +import { getBitbucketReviewGrantStatus } from '@kilocode/worker-utils/bitbucket-workspace-access-token'; import { + BitbucketOAuthScopeError, exchangeBitbucketOAuthCode, fetchBitbucketUser, fetchBitbucketWorkspaces, @@ -17,6 +19,7 @@ import { import { BitbucketIntegrationAuthorizationError, BitbucketIntegrationConnectionConflictError, + BitbucketIntegrationRecoveryError, storeBitbucketIntegration, } from '@/lib/integrations/platforms/bitbucket/credentials'; import { scheduleBitbucketRepositoryCachePrime } from '@/lib/integrations/platforms/bitbucket/repository-cache'; @@ -103,8 +106,21 @@ export async function handleBitbucketOAuthCallback(request: NextRequest): Promis return redirectWithStatus(verifiedState, 'error', 'unauthorized'); } - if (searchParams.get('error')) { - return redirectWithStatus(verifiedState, 'error', 'authorization_cancelled'); + // Only the start handler can bind replacement to the signed state. + if (searchParams.has('reconnectIntegrationId')) { + return redirectWithStatus(verifiedState, 'error', 'invalid_state'); + } + const providerError = searchParams.get('error'); + if (providerError) { + return redirectWithStatus( + verifiedState, + 'error', + !verifiedState.bitbucketRecovery || providerError === 'access_denied' + ? 'authorization_cancelled' + : providerError === 'invalid_scope' + ? 'missing_scopes' + : 'connection_failed' + ); } const code = validOAuthCode(searchParams.get('code')); @@ -114,13 +130,23 @@ export async function handleBitbucketOAuthCallback(request: NextRequest): Promis callbackPhase = 'token_exchange'; const tokens = await exchangeBitbucketOAuthCode(code); + if ( + verifiedState.bitbucketRecovery && + !getBitbucketReviewGrantStatus(tokens.scopes, 'oauth').writeReady + ) { + return redirectWithStatus(verifiedState, 'error', 'missing_scopes'); + } callbackPhase = 'provider_profile'; const [bitbucketUser, availableWorkspaces] = await Promise.all([ fetchBitbucketUser(tokens.accessToken), fetchBitbucketWorkspaces(tokens.accessToken), ]); if (availableWorkspaces.length === 0) { - return redirectWithStatus(verifiedState, 'error', 'no_workspaces'); + return redirectWithStatus( + verifiedState, + 'error', + verifiedState.bitbucketRecovery ? 'workspace_unavailable' : 'no_workspaces' + ); } callbackPhase = 'store_integration'; @@ -130,8 +156,9 @@ export async function handleBitbucketOAuthCallback(request: NextRequest): Promis bitbucketUser, tokens, availableWorkspaces, + bitbucketRecovery: verifiedState.bitbucketRecovery, }); - if (storedIntegration.status === 'connected') { + if (storedIntegration.status === 'connected' && !verifiedState.bitbucketRecovery) { scheduleBitbucketRepositoryCachePrime({ owner, kiloUserId: user.id, @@ -147,6 +174,12 @@ export async function handleBitbucketOAuthCallback(request: NextRequest): Promis if (error instanceof BitbucketIntegrationConnectionConflictError) { return redirectWithStatus(verifiedState, 'error', 'connection_exists'); } + if (error instanceof BitbucketIntegrationRecoveryError) { + return redirectWithStatus(verifiedState, 'error', error.code); + } + if (verifiedState?.bitbucketRecovery && error instanceof BitbucketOAuthScopeError) { + return redirectWithStatus(verifiedState, 'error', 'missing_scopes'); + } const callbackContext = safeCallbackContext(searchParams); if (process.env.NODE_ENV === 'development') { diff --git a/apps/web/src/lib/integrations/oauth/platforms/bitbucket-connect.ts b/apps/web/src/lib/integrations/oauth/platforms/bitbucket-connect.ts index e9dd121525..b3d737c87e 100644 --- a/apps/web/src/lib/integrations/oauth/platforms/bitbucket-connect.ts +++ b/apps/web/src/lib/integrations/oauth/platforms/bitbucket-connect.ts @@ -5,9 +5,14 @@ import { APP_URL } from '@/lib/constants'; import { PLATFORM } from '@/lib/integrations/core/constants'; import type { Owner } from '@/lib/integrations/core/types'; import { buildBitbucketOAuthUrl } from '@/lib/integrations/platforms/bitbucket/adapter'; +import { + BitbucketIntegrationRecoveryError, + getBitbucketOAuthRecovery, +} from '@/lib/integrations/platforms/bitbucket/credentials'; import { createOAuthState } from '@/lib/integrations/oauth-state'; import { buildIntegrationOAuthConnectErrorPath, + organizationAccessDenialErrorCode, redirectToSignInForOAuthConnect, } from '@/lib/integrations/oauth/common'; import { validateReturnPath } from '@/lib/integrations/validate-return-path'; @@ -37,22 +42,35 @@ export async function handleBitbucketOAuthConnect(request: NextRequest): Promise await ensureOrganizationAccess({ user }, owner.id, ORGANIZATION_BILLING_ROLES); } + const reconnectIntegrationId = request.nextUrl.searchParams.get('reconnectIntegrationId'); + const recovery = + reconnectIntegrationId !== null + ? await getBitbucketOAuthRecovery(owner, reconnectIntegrationId) + : undefined; const returnToParam = request.nextUrl.searchParams.get('returnTo'); const returnTo = returnToParam ? validateReturnPath(returnToParam) : null; - const state = createOAuthState(`${owner.type}_${owner.id}`, user.id, returnTo ?? undefined); + const state = createOAuthState( + `${owner.type}_${owner.id}`, + user.id, + returnTo ?? undefined, + recovery + ); return NextResponse.redirect(buildBitbucketOAuthUrl(state)); } catch (error) { captureException(error, { tags: { endpoint: 'bitbucket/connect', source: 'bitbucket_oauth' }, extra: { hasOrganizationId: Boolean(organizationId) }, }); + const errorCode = + error instanceof BitbucketIntegrationRecoveryError + ? error.code + : request.nextUrl.searchParams.has('reconnectIntegrationId') && + organizationAccessDenialErrorCode(error) + ? 'unauthorized' + : 'oauth_init_failed'; return NextResponse.redirect( new URL( - buildIntegrationOAuthConnectErrorPath( - PLATFORM.BITBUCKET, - organizationId, - 'oauth_init_failed' - ), + buildIntegrationOAuthConnectErrorPath(PLATFORM.BITBUCKET, organizationId, errorCode), APP_URL ) ); diff --git a/apps/web/src/lib/integrations/platforms/bitbucket/adapter.test.ts b/apps/web/src/lib/integrations/platforms/bitbucket/adapter.test.ts index 945552c49a..56e49a8850 100644 --- a/apps/web/src/lib/integrations/platforms/bitbucket/adapter.test.ts +++ b/apps/web/src/lib/integrations/platforms/bitbucket/adapter.test.ts @@ -3,7 +3,9 @@ jest.mock('@/lib/config.server', () => ({ BITBUCKET_CLIENT_SECRET: 'bitbucket-client-secret', })); +import { getBitbucketReviewGrantStatus } from '@kilocode/worker-utils/bitbucket-workspace-access-token'; import { + BitbucketOAuthScopeError, buildBitbucketOAuthUrl, exchangeBitbucketOAuthCode, fetchBitbucketUser, @@ -32,7 +34,7 @@ describe('Bitbucket OAuth adapter', () => { expect(Object.fromEntries(url.searchParams)).toEqual({ client_id: 'bitbucket-client-id', response_type: 'code', - scope: 'account repository:write pullrequest webhook', + scope: 'account repository:write pullrequest webhook pullrequest:write', state: 'signed-state', }); expect(url.toString()).not.toContain('bitbucket-client-secret'); @@ -42,13 +44,19 @@ describe('Bitbucket OAuth adapter', () => { const authorizationCode = 'authorization code+&='; const fetchMock = jest.spyOn(global, 'fetch').mockResolvedValueOnce(validTokenResponse()); - await expect(exchangeBitbucketOAuthCode(authorizationCode)).resolves.toEqual({ + const tokens = await exchangeBitbucketOAuthCode(authorizationCode); + expect(tokens).toEqual({ accessToken: 'access-token', refreshToken: 'refresh-token', tokenType: 'bearer', expiresIn: 3600, scopes: ['account', 'email', 'pullrequest', 'repository', 'repository:write', 'webhook'], }); + expect(getBitbucketReviewGrantStatus(tokens.scopes, 'oauth')).toEqual({ + readReady: true, + writeReady: false, + recoveryAction: 'reconnect', + }); expect(fetchMock).toHaveBeenCalledTimes(1); const [url, init] = fetchMock.mock.calls[0] ?? []; @@ -109,10 +117,10 @@ describe('Bitbucket OAuth adapter', () => { ); }); - it('accepts the transitional plural scopes field alongside canonical scope', async () => { + it('does not upgrade read grants from the transitional plural scopes field', async () => { jest.spyOn(global, 'fetch').mockResolvedValueOnce( validTokenResponse({ - scopes: 'repository:write repository account pullrequest webhook', + scopes: 'account pullrequest:write webhook', }) ); @@ -146,6 +154,30 @@ describe('Bitbucket OAuth adapter', () => { ); }); + it.each(['pullrequest:write', 'write:pullrequest:bitbucket-legacy'])( + 'retains %s after OAuth reconnect without requiring it on old grants', + async grant => { + jest + .spyOn(global, 'fetch') + .mockResolvedValueOnce(validTokenResponse({ scope: `account ${grant} webhook` })); + const tokens = await exchangeBitbucketOAuthCode('reconnect-code'); + expect(tokens.scopes).toEqual([ + 'account', + 'email', + 'pullrequest', + 'pullrequest:write', + 'repository', + 'repository:write', + 'webhook', + ]); + expect(getBitbucketReviewGrantStatus(tokens.scopes, 'oauth')).toEqual({ + readReady: true, + writeReady: true, + recoveryAction: null, + }); + } + ); + it('rejects the retired plural scopes response field without canonical scope', async () => { jest .spyOn(global, 'fetch') @@ -166,9 +198,11 @@ describe('Bitbucket OAuth adapter', () => { ])('rejects token responses missing a required OAuth scope', async scope => { jest.spyOn(global, 'fetch').mockResolvedValueOnce(validTokenResponse({ scope })); - await expect(exchangeBitbucketOAuthCode('authorization-code')).rejects.toThrow( + const exchange = exchangeBitbucketOAuthCode('authorization-code'); + await expect(exchange).rejects.toThrow( 'Bitbucket OAuth token exchange returned invalid credentials' ); + await expect(exchange).rejects.toBeInstanceOf(BitbucketOAuthScopeError); }); it('ignores token response scopes beyond the required OAuth grant', async () => { diff --git a/apps/web/src/lib/integrations/platforms/bitbucket/adapter.ts b/apps/web/src/lib/integrations/platforms/bitbucket/adapter.ts index f72c4decae..611bec4fa3 100644 --- a/apps/web/src/lib/integrations/platforms/bitbucket/adapter.ts +++ b/apps/web/src/lib/integrations/platforms/bitbucket/adapter.ts @@ -67,12 +67,18 @@ const BitbucketWorkspacePageSchema = z.object({ next: z.string().min(1).optional(), }); -export const BITBUCKET_OAUTH_SCOPES = [ +// Preserve old read grants until old clients/records and the 30-day ledger window expire. +const BITBUCKET_OAUTH_READ_SCOPES = [ 'account', 'repository:write', 'pullrequest', 'webhook', ] as const; +// Deployment requires Pull request Write on the OAuth consumer; requesting it cannot grant it. +export const BITBUCKET_OAUTH_SCOPES = [ + ...BITBUCKET_OAUTH_READ_SCOPES, + 'pullrequest:write', +] as const; const BITBUCKET_OAUTH_SCOPE_ALIASES: Record = { 'read:account:bitbucket-legacy': ['account'], @@ -84,11 +90,16 @@ const BITBUCKET_OAUTH_SCOPE_ALIASES: Record = { 'admin:webhook:bitbucket-legacy': ['webhook'], pullrequest: ['pullrequest'], 'read:pullrequest:bitbucket-legacy': ['pullrequest'], + 'write:pullrequest:bitbucket-legacy': ['pullrequest:write'], offline_access: [], }; function expandBitbucketOAuthScopeClosure(scopes: Iterable): Set { const closure = new Set(scopes); + if (closure.has('pullrequest:write')) { + closure.add('pullrequest'); + closure.add('repository:write'); + } if (closure.has('repository:write')) { closure.add('repository'); } @@ -98,6 +109,8 @@ function expandBitbucketOAuthScopeClosure(scopes: Iterable): Set return closure; } +export class BitbucketOAuthScopeError extends Error {} + function normalizeBitbucketOAuthScopes(scope: string): string[] { const canonicalScopes = new Set(); for (const rawScope of scope.split(/\s+/).filter(Boolean)) { @@ -110,11 +123,11 @@ function normalizeBitbucketOAuthScopes(scope: string): string[] { const returnedScopes = expandBitbucketOAuthScopeClosure(canonicalScopes); const allowedScopes = expandBitbucketOAuthScopeClosure(BITBUCKET_OAUTH_SCOPES); - if (BITBUCKET_OAUTH_SCOPES.some(requiredScope => !returnedScopes.has(requiredScope))) { - const missingScopes = BITBUCKET_OAUTH_SCOPES.filter( + if (BITBUCKET_OAUTH_READ_SCOPES.some(requiredScope => !returnedScopes.has(requiredScope))) { + const missingScopes = BITBUCKET_OAUTH_READ_SCOPES.filter( requiredScope => !returnedScopes.has(requiredScope) ); - throw new Error( + throw new BitbucketOAuthScopeError( `Bitbucket OAuth token exchange returned invalid credentials: scope_mismatch missing=${missingScopes.join(',') || 'none'} observed=${[...returnedScopes].sort().join(',') || 'none'}` ); } diff --git a/apps/web/src/lib/integrations/platforms/bitbucket/credentials-reconnect.test.ts b/apps/web/src/lib/integrations/platforms/bitbucket/credentials-reconnect.test.ts new file mode 100644 index 0000000000..77ed09f565 --- /dev/null +++ b/apps/web/src/lib/integrations/platforms/bitbucket/credentials-reconnect.test.ts @@ -0,0 +1,470 @@ +import { generateKeyPairSync } from 'node:crypto'; +import { decryptKeyedEnvelope } from '@kilocode/encryption'; +import { PgDialect } from 'drizzle-orm/pg-core'; +import type { SQL } from 'drizzle-orm'; +import { db } from '@/lib/drizzle'; +import { + kilocode_users, + organization_memberships, + platform_integrations, + platform_oauth_credentials, + type PlatformIntegration, + type PlatformOAuthCredential, +} from '@kilocode/db/schema'; +import { + BITBUCKET_OAUTH_CREDENTIAL_ENVELOPE_SCHEME, + BitbucketIntegrationAuthorizationError, + BitbucketIntegrationConnectionConflictError, + BitbucketIntegrationRecoveryError, + buildBitbucketOAuthCredentialAad, + getBitbucketOAuthRecovery, + storeBitbucketIntegration, + type StoreBitbucketIntegrationInput, +} from './credentials'; + +jest.mock('@/lib/drizzle', () => ({ db: { transaction: jest.fn(), select: jest.fn() } })); +jest.mock('@/lib/config.server', () => ({ + get BITBUCKET_OAUTH_CREDENTIAL_ACTIVE_KEY_ID() { + return 'recovery-test-key'; + }, + get BITBUCKET_OAUTH_CREDENTIAL_ACTIVE_PUBLIC_KEY() { + return mockPublicKey; + }, +})); + +const keyPair = generateKeyPairSync('rsa', { + modulusLength: 2048, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, +}); +let mockPublicKey = Buffer.from(keyPair.publicKey).toString('base64'); +const dialect = new PgDialect(); +const owner = { type: 'org', id: '77777777-7777-4777-8777-777777777777' } as const; +const actor = 'oauth/manager'; +const recovery = { + integrationId: '33333333-3333-4333-8333-333333333333', + credentialId: '44444444-4444-4444-8444-444444444444', + credentialVersion: 2, + workspaceUuid: 'workspace-one', + workspaceSlug: 'workspace-one', +}; +const ownerLock = `bitbucket-oauth-owner:org:${owner.id}`; +const credentialLock = `bitbucket-oauth-credential:${recovery.credentialId}`; + +type Stored = { + integration: PlatformIntegration | undefined; + credential: PlatformOAuthCredential | undefined; +}; +let stored: Stored; +let authorizerExists: boolean; +let isAdmin: boolean; +let membershipExists: boolean; +let failUpdate: 'credential' | 'integration' | undefined; +let requiredLocks: string[]; + +function input(): StoreBitbucketIntegrationInput { + return { + owner, + authorizedByUserId: actor, + bitbucketRecovery: { ...recovery }, + bitbucketUser: { uuid: '{new-bot}', nickname: 'new-bot' }, + tokens: { + accessToken: 'new-access-token', + refreshToken: 'new-refresh-token', + tokenType: 'bearer', + expiresIn: 3600, + scopes: ['account', 'pullrequest:write', 'webhook'], + }, + availableWorkspaces: [ + { uuid: '{other-workspace}', slug: 'other-workspace', name: 'Other' }, + { uuid: '{WORKSPACE-ONE}', slug: 'workspace-one', name: 'Renamed provider label' }, + ], + }; +} + +beforeEach(() => { + jest.clearAllMocks(); + mockPublicKey = Buffer.from(keyPair.publicKey).toString('base64'); + authorizerExists = true; + isAdmin = false; + membershipExists = true; + failUpdate = undefined; + requiredLocks = [ownerLock, credentialLock]; + stored = { + integration: { + id: recovery.integrationId, + owned_by_organization_id: owner.id, + owned_by_user_id: null, + platform: 'bitbucket', + integration_type: 'oauth', + integration_status: 'active', + platform_installation_id: recovery.workspaceUuid, + platform_account_id: recovery.workspaceUuid, + platform_account_login: recovery.workspaceSlug, + metadata: { + state: 'active', + workspace: { + uuid: recovery.workspaceUuid, + slug: recovery.workspaceSlug, + name: 'Original workspace', + }, + }, + scopes: ['account', 'pullrequest', 'repository:write', 'webhook'], + repositories: [ + { id: 'repository-one', name: 'mobile', full_name: 'workspace-one/mobile', private: true }, + ], + repositories_synced_at: '2026-08-30 09:00:00+00', + } as PlatformIntegration, + credential: { + id: recovery.credentialId, + platform_integration_id: recovery.integrationId, + authorized_by_user_id: actor, + provider_subject_id: 'old-bot', + provider_subject_login: 'old-bot', + access_token_encrypted: 'old-access-envelope', + refresh_token_encrypted: 'old-refresh-envelope', + credential_version: recovery.credentialVersion, + revoked_at: null, + revocation_reason: null, + } as PlatformOAuthCredential, + }; + + jest.mocked(db.select).mockImplementation( + () => + ({ + from: () => ({ + innerJoin: () => ({ + where: (condition: SQL) => ({ + limit: async () => { + const query = dialect.sqlToQuery(condition); + expect(query.sql).toContain('"owned_by_organization_id"'); + expect(query.params).toEqual(expect.arrayContaining([owner.id, 'bitbucket'])); + return stored.integration && + stored.credential && + query.params.includes(stored.integration.id) + ? [{ integration: stored.integration, credential: stored.credential }] + : []; + }, + }), + }), + }), + }) as never + ); + + // Stage writes separately so the test observes commit and rollback, not just calls. + jest.mocked(db.transaction).mockImplementation(async callback => { + const before = structuredClone(stored); + const staged = structuredClone(stored); + const locks = new Set(); + const rowLocks = new Set(); + const tx = { + execute: async (statement: SQL) => { + const query = dialect.sqlToQuery(statement); + expect(query.sql).toContain('pg_advisory_xact_lock'); + for (const value of query.params) if (typeof value === 'string') locks.add(value); + }, + select: () => ({ + from: (table: unknown) => ({ + where: (condition: SQL) => ({ + for: async (lock: string) => { + expect(lock).toBe('update'); + for (const key of requiredLocks) expect(locks.has(key)).toBe(true); + rowLocks.add(table); + const query = dialect.sqlToQuery(condition); + if (table === kilocode_users) { + expect(query.sql).toContain('"blocked_reason" is null'); + expect(query.params).toContain(actor); + return authorizerExists ? [{ isAdmin }] : []; + } + if (table === organization_memberships) { + expect(query.params).toEqual( + expect.arrayContaining([owner.id, actor, 'owner', 'billing_manager']) + ); + return membershipExists ? [{ id: 'membership' }] : []; + } + if (table === platform_integrations) { + expect(query.sql).toContain('"owned_by_organization_id"'); + expect(query.params).toEqual(expect.arrayContaining([owner.id, 'bitbucket'])); + return staged.integration ? [staged.integration] : []; + } + if (table === platform_oauth_credentials) { + expect(query.params).toContain(recovery.integrationId); + return staged.credential ? [staged.credential] : []; + } + throw new Error('Unexpected table'); + }, + }), + }), + }), + update: (table: unknown) => ({ + set: (values: object) => ({ + where: (condition: SQL) => ({ + returning: async () => { + expect(rowLocks.has(table)).toBe(true); + expect(stored).toEqual(before); + const query = dialect.sqlToQuery(condition); + if (table === platform_oauth_credentials) { + expect(query.params).toEqual( + expect.arrayContaining([ + recovery.credentialId, + recovery.integrationId, + recovery.credentialVersion, + ]) + ); + if (failUpdate === 'credential') throw new Error('Credential write failed'); + staged.credential = { ...staged.credential, ...values } as PlatformOAuthCredential; + return [{ id: staged.credential.id }]; + } + if (table === platform_integrations) { + expect(query.params).toContain(recovery.integrationId); + if (failUpdate === 'integration') throw new Error('Integration write failed'); + staged.integration = { ...staged.integration, ...values } as PlatformIntegration; + return [{ id: staged.integration.id }]; + } + throw new Error('Unexpected update'); + }, + }), + }), + }), + }; + const result = await callback(tx as never); + stored = staged; + return result; + }); +}); + +describe('Bitbucket OAuth recovery transaction', () => { + it('derives recovery from the exact owned connection', async () => { + await expect(getBitbucketOAuthRecovery(owner, recovery.integrationId)).resolves.toEqual( + recovery + ); + await expect( + getBitbucketOAuthRecovery(owner, '55555555-5555-4555-8555-555555555555') + ).rejects.toThrow(BitbucketIntegrationRecoveryError); + }); + + it('atomically replaces encrypted credentials while preserving the selected workspace and cache', async () => { + const before = structuredClone(stored); + const result = await storeBitbucketIntegration(input()); + expect(result).toEqual({ status: 'connected', integrationId: recovery.integrationId }); + expect(stored.integration).toMatchObject({ + id: recovery.integrationId, + metadata: before.integration?.metadata, + platform_account_id: recovery.workspaceUuid, + platform_account_login: recovery.workspaceSlug, + repositories: before.integration?.repositories, + repositories_synced_at: before.integration?.repositories_synced_at, + scopes: input().tokens.scopes, + }); + expect(stored.credential).toMatchObject({ + id: recovery.credentialId, + credential_version: recovery.credentialVersion + 1, + provider_subject_id: 'new-bot', + provider_subject_login: 'new-bot', + }); + for (const kind of ['access', 'refresh'] as const) { + const envelope = stored.credential?.[`${kind}_token_encrypted`]; + expect( + decryptKeyedEnvelope( + envelope ?? '', + BITBUCKET_OAUTH_CREDENTIAL_ENVELOPE_SCHEME, + { active: { keyId: 'recovery-test-key', privateKeyPem: keyPair.privateKey } }, + buildBitbucketOAuthCredentialAad({ + credentialId: recovery.credentialId, + integrationId: recovery.integrationId, + owner, + authorizedByUserId: actor, + kind, + }) + ) + ).toBe(`new-${kind}-token`); + } + }); + + it.each(['credential', 'integration'] as const)( + 'rolls back both rows when the %s write fails', + async table => { + failUpdate = table; + const before = structuredClone(stored); + await expect(storeBitbucketIntegration(input())).rejects.toThrow('write failed'); + expect(stored).toEqual(before); + } + ); + + it('rejects replay after a successful replacement without undoing that replacement', async () => { + await storeBitbucketIntegration(input()); + const replaced = structuredClone(stored); + await expect(storeBitbucketIntegration(input())).rejects.toThrow( + BitbucketIntegrationRecoveryError + ); + expect(stored).toEqual(replaced); + }); + + it.each([ + [ + 'disconnected', + (value: Stored) => { + value.integration = undefined; + }, + ], + [ + 'missing credential', + (value: Stored) => { + value.credential = undefined; + }, + ], + [ + 'different integration', + (value: Stored) => { + if (value.integration) value.integration.id = '55555555-5555-4555-8555-555555555555'; + }, + ], + [ + 'different method', + (value: Stored) => { + if (value.integration) value.integration.integration_type = 'workspace_access_token'; + }, + ], + [ + 'inactive integration', + (value: Stored) => { + if (value.integration) value.integration.integration_status = 'suspended'; + }, + ], + [ + 'different workspace UUID', + (value: Stored) => { + if (value.integration) value.integration.platform_account_id = 'workspace-other'; + }, + ], + [ + 'different workspace slug', + (value: Stored) => { + if (value.integration) value.integration.platform_account_login = 'workspace-other'; + }, + ], + [ + 'changed metadata', + (value: Stored) => { + if (value.integration) + value.integration.metadata = { + state: 'workspace_selection_required', + availableWorkspaces: [], + }; + }, + ], + [ + 'different credential', + (value: Stored) => { + if (value.credential) value.credential.id = '66666666-6666-4666-8666-666666666666'; + }, + ], + [ + 'newer credential', + (value: Stored) => { + if (value.credential) value.credential.credential_version += 1; + }, + ], + [ + 'revoked credential', + (value: Stored) => { + if (value.credential) value.credential.revoked_at = '2026-08-30 09:00:00+00'; + }, + ], + ] as const)('retains all current data when recovery is stale: %s', async (_, change) => { + change(stored); + const before = structuredClone(stored); + await expect(storeBitbucketIntegration(input())).rejects.toThrow( + BitbucketIntegrationRecoveryError + ); + expect(stored).toEqual(before); + }); + + it.each([ + { reason: 'no workspaces', availableWorkspaces: [] }, + { + reason: 'different UUID', + availableWorkspaces: [ + { uuid: '{workspace-other}', slug: 'workspace-one', name: 'Wrong UUID' }, + ], + }, + { + reason: 'different slug', + availableWorkspaces: [ + { uuid: '{workspace-one}', slug: 'workspace-other', name: 'Wrong slug' }, + ], + }, + ])( + 'does not select another workspace when recovery has $reason', + async ({ availableWorkspaces }) => { + const before = structuredClone(stored); + await expect( + storeBitbucketIntegration({ ...input(), availableWorkspaces }) + ).rejects.toMatchObject({ code: 'workspace_unavailable' }); + expect(stored).toEqual(before); + } + ); + + it.each([ + { missing: 'write grants', scopes: ['account', 'repository:write', 'pullrequest', 'webhook'] }, + { missing: 'account grants', scopes: ['pullrequest:write', 'webhook'] }, + { missing: 'webhook grants', scopes: ['account', 'pullrequest:write'] }, + ])('retains read credentials when recovery lacks $missing', async ({ scopes }) => { + const before = structuredClone(stored); + const replacement = input(); + replacement.tokens.scopes = scopes; + await expect(storeBitbucketIntegration(replacement)).rejects.toMatchObject({ + code: 'missing_scopes', + }); + expect(stored).toEqual(before); + }); + + it.each(['authorizer', 'membership'] as const)( + 'rechecks current %s access inside the transaction', + async missing => { + authorizerExists = missing !== 'authorizer'; + membershipExists = missing !== 'membership'; + const before = structuredClone(stored); + await expect(storeBitbucketIntegration(input())).rejects.toThrow( + BitbucketIntegrationAuthorizationError + ); + expect(stored).toEqual(before); + } + ); + + it('permits a current platform admin without organization membership', async () => { + isAdmin = true; + membershipExists = false; + await expect(storeBitbucketIntegration(input())).resolves.toEqual({ + status: 'connected', + integrationId: recovery.integrationId, + }); + expect(stored.credential?.credential_version).toBe(3); + }); + + it('rejects a personal owner mismatch before persisting credentials', async () => { + const before = structuredClone(stored); + await expect( + storeBitbucketIntegration({ ...input(), owner: { type: 'user', id: 'oauth/someone-else' } }) + ).rejects.toThrow(BitbucketIntegrationAuthorizationError); + expect(stored).toEqual(before); + }); + + it('retains the old connection when encryption is unavailable', async () => { + mockPublicKey = ''; + const before = structuredClone(stored); + await expect(storeBitbucketIntegration(input())).rejects.toThrow( + 'encryption is not configured' + ); + expect(stored).toEqual(before); + }); + + it('keeps the legacy first-connect conflict instead of replacing an existing connection', async () => { + requiredLocks = [ownerLock]; + const before = structuredClone(stored); + await expect( + storeBitbucketIntegration({ ...input(), bitbucketRecovery: undefined }) + ).rejects.toThrow(BitbucketIntegrationConnectionConflictError); + expect(stored).toEqual(before); + }); +}); diff --git a/apps/web/src/lib/integrations/platforms/bitbucket/credentials.test.ts b/apps/web/src/lib/integrations/platforms/bitbucket/credentials.test.ts index 2d41601bd3..84f45a24da 100644 --- a/apps/web/src/lib/integrations/platforms/bitbucket/credentials.test.ts +++ b/apps/web/src/lib/integrations/platforms/bitbucket/credentials.test.ts @@ -16,6 +16,8 @@ import { and, eq } from 'drizzle-orm'; import { BITBUCKET_OAUTH_CREDENTIAL_ENVELOPE_SCHEME, BitbucketIntegrationConnectionConflictError, + BitbucketIntegrationRecoveryError, + getBitbucketOAuthRecovery, buildBitbucketOAuthCredentialAad, storeBitbucketIntegration, } from './credentials'; @@ -104,6 +106,52 @@ async function insertExistingBitbucketIntegration(kiloUserId: string) { return { integration, credential }; } +async function readStoredIntegration(integrationId: string) { + const [integration] = await db + .select() + .from(platform_integrations) + .where(eq(platform_integrations.id, integrationId)); + const [credential] = await db + .select() + .from(platform_oauth_credentials) + .where(eq(platform_oauth_credentials.platform_integration_id, integrationId)); + return { integration, credential }; +} + +async function recoveryFixture() { + const user = await insertTestUser(); + const owner = { type: 'user', id: user.id } as const; + const { integration } = await insertExistingBitbucketIntegration(user.id); + await db + .update(platform_integrations) + .set({ + repositories: [ + { + id: '88888888-8888-4888-8888-888888888888', + name: 'mobile', + full_name: 'workspace-old/mobile', + private: true, + }, + ], + repositories_synced_at: '2026-08-30T09:00:00.000Z', + }) + .where(eq(platform_integrations.id, integration.id)); + const replacement = integrationInput(owner, user.id); + replacement.bitbucketRecovery = await getBitbucketOAuthRecovery(owner, integration.id); + replacement.tokens.scopes.push('pullrequest:write'); + replacement.availableWorkspaces = [ + { uuid: '{workspace-other}', slug: 'workspace-other', name: 'Other Workspace' }, + { uuid: '{WORKSPACE-OLD}', slug: 'workspace-old', name: 'Changed display name' }, + ]; + return { + user, + owner, + replacement, + before: await readStoredIntegration(integration.id), + integrationId: integration.id, + }; +} + describe('Bitbucket OAuth credential storage', () => { beforeEach(() => { mockBitbucketCredentialEncryptionConfig.keyId = 'bitbucket-credential-key-v1'; @@ -309,11 +357,23 @@ describe('Bitbucket OAuth credential storage', () => { expect(credentials[0]?.id).toBe(existing.credential.id); }); - it('preserves an organization integration when callback authorization was revoked', async () => { + it('preserves organization credentials when recovery authorization was revoked', async () => { const user = await insertTestUser(); const organization = await createTestOrganization('Revoked Callback Org', user.id, 0); const owner = { type: 'org', id: organization.id } as const; - const existing = await storeBitbucketIntegration(integrationInput(owner, user.id, 'existing')); + const original = integrationInput(owner, user.id, 'existing'); + const existing = await storeBitbucketIntegration(original); + const replacement = { + ...original, + tokens: { + ...original.tokens, + accessToken: 'replacement-access', + refreshToken: 'replacement-refresh', + scopes: [...original.tokens.scopes, 'pullrequest:write'], + }, + bitbucketRecovery: await getBitbucketOAuthRecovery(owner, existing.integrationId), + }; + const before = await readStoredIntegration(existing.integrationId); await db .delete(organization_memberships) @@ -324,15 +384,8 @@ describe('Bitbucket OAuth credential storage', () => { ) ); - await expect( - storeBitbucketIntegration(integrationInput(owner, user.id, 'replacement')) - ).rejects.toThrow('no longer authorized'); - await expect( - db - .select({ id: platform_integrations.id }) - .from(platform_integrations) - .where(eq(platform_integrations.owned_by_organization_id, organization.id)) - ).resolves.toEqual([{ id: existing.integrationId }]); + await expect(storeBitbucketIntegration(replacement)).rejects.toThrow('no longer authorized'); + expect(await readStoredIntegration(existing.integrationId)).toEqual(before); }); it('rechecks current platform-admin access inside the storage transaction', async () => { @@ -362,6 +415,172 @@ describe('Bitbucket OAuth credential storage', () => { ).resolves.toEqual([{ id: existing.integrationId }]); }); + it('recovers write grants without changing integration identity, workspace, or cached repositories', async () => { + const fixture = await recoveryFixture(); + const result = await storeBitbucketIntegration(fixture.replacement); + const after = await readStoredIntegration(fixture.integrationId); + expect(result).toEqual({ status: 'connected', integrationId: fixture.integrationId }); + expect(after.integration).toEqual({ + ...fixture.before.integration, + scopes: fixture.replacement.tokens.scopes, + updated_at: after.integration?.updated_at, + }); + expect(after.credential).toMatchObject({ + id: fixture.before.credential?.id, + platform_integration_id: fixture.integrationId, + credential_version: 2, + provider_subject_login: fixture.replacement.bitbucketUser.nickname, + }); + if (!after.credential) throw new Error('Expected recovered credential'); + for (const kind of ['access', 'refresh'] as const) { + expect( + decryptKeyedEnvelope( + after.credential[`${kind}_token_encrypted`] ?? '', + BITBUCKET_OAUTH_CREDENTIAL_ENVELOPE_SCHEME, + { + active: { + keyId: mockBitbucketCredentialEncryptionConfig.keyId, + privateKeyPem: testKeyPair.privateKey, + }, + }, + buildBitbucketOAuthCredentialAad({ + credentialId: after.credential.id, + integrationId: fixture.integrationId, + owner: fixture.owner, + authorizedByUserId: fixture.user.id, + kind, + }) + ) + ).toBe(fixture.replacement.tokens[kind === 'access' ? 'accessToken' : 'refreshToken']); + } + }); + + it('rolls back a credential update when the following integration write fails', async () => { + const fixture = await recoveryFixture(); + const transaction = db.transaction.bind(db); + const transactionSpy = jest.spyOn(db, 'transaction').mockImplementation(callback => + transaction(async tx => { + const update = tx.update.bind(tx); + const failIntegrationUpdate: typeof tx.update = table => { + if (table === platform_integrations) throw new Error('forced integration write failure'); + return update(table); + }; + jest.spyOn(tx, 'update').mockImplementation(failIntegrationUpdate); + return callback(tx); + }) + ); + try { + await expect(storeBitbucketIntegration(fixture.replacement)).rejects.toThrow( + 'forced integration write failure' + ); + } finally { + transactionSpy.mockRestore(); + } + expect(await readStoredIntegration(fixture.integrationId)).toEqual(fixture.before); + }); + + it.each(['workspace', 'credential revision', 'disconnection'])( + 'rejects recovery after a changed %s without modifying current rows', + async change => { + const fixture = await recoveryFixture(); + if (change === 'workspace') { + await db + .update(platform_integrations) + .set({ + platform_account_id: 'changed-workspace', + platform_installation_id: 'changed-workspace', + platform_account_login: 'changed-workspace', + metadata: { + state: 'active', + workspace: { + uuid: 'changed-workspace', + slug: 'changed-workspace', + name: 'Changed Workspace', + }, + }, + }) + .where(eq(platform_integrations.id, fixture.integrationId)); + } else if (change === 'credential revision') { + await db + .update(platform_oauth_credentials) + .set({ credential_version: 2 }) + .where(eq(platform_oauth_credentials.platform_integration_id, fixture.integrationId)); + } else { + await db + .delete(platform_integrations) + .where(eq(platform_integrations.id, fixture.integrationId)); + } + const current = await readStoredIntegration(fixture.integrationId); + await expect(storeBitbucketIntegration(fixture.replacement)).rejects.toThrow( + BitbucketIntegrationRecoveryError + ); + expect(await readStoredIntegration(fixture.integrationId)).toEqual(current); + } + ); + + it.each(['workspace', 'write grants'])( + 'retains credentials and cache when recovery lacks %s', + async missing => { + const fixture = await recoveryFixture(); + if (missing === 'workspace') fixture.replacement.availableWorkspaces = []; + else + fixture.replacement.tokens.scopes = [ + 'account', + 'repository:write', + 'pullrequest', + 'webhook', + ]; + await expect(storeBitbucketIntegration(fixture.replacement)).rejects.toMatchObject({ + code: missing === 'workspace' ? 'workspace_unavailable' : 'missing_scopes', + }); + expect(await readStoredIntegration(fixture.integrationId)).toEqual(fixture.before); + } + ); + + it('rejects a recovery target from a different owner without changing either connection', async () => { + const first = await recoveryFixture(); + const second = await recoveryFixture(); + await expect(getBitbucketOAuthRecovery(second.owner, first.integrationId)).rejects.toThrow( + BitbucketIntegrationRecoveryError + ); + await expect( + storeBitbucketIntegration({ + ...second.replacement, + bitbucketRecovery: first.replacement.bitbucketRecovery, + }) + ).rejects.toThrow(BitbucketIntegrationRecoveryError); + expect(await readStoredIntegration(first.integrationId)).toEqual(first.before); + expect(await readStoredIntegration(second.integrationId)).toEqual(second.before); + }); + + it('rechecks a blocked personal authorizer inside recovery persistence', async () => { + const fixture = await recoveryFixture(); + await db + .update(kilocode_users) + .set({ blocked_reason: 'blocked during OAuth recovery' }) + .where(eq(kilocode_users.id, fixture.user.id)); + await expect(storeBitbucketIntegration(fixture.replacement)).rejects.toThrow( + 'no longer authorized' + ); + expect(await readStoredIntegration(fixture.integrationId)).toEqual(fixture.before); + }); + + it('allows only one commit for competing callbacks with the same signed recovery revision', async () => { + const fixture = await recoveryFixture(); + const results = await Promise.allSettled([ + storeBitbucketIntegration(fixture.replacement), + storeBitbucketIntegration(fixture.replacement), + ]); + expect(results.map(result => result.status).sort()).toEqual(['fulfilled', 'rejected']); + const after = await readStoredIntegration(fixture.integrationId); + expect(after.credential?.credential_version).toBe(2); + expect(after.integration?.metadata).toEqual(fixture.before.integration?.metadata); + expect(after.integration?.repositories).toEqual(fixture.before.integration?.repositories); + expect(after.integration?.repositories_synced_at).toEqual( + fixture.before.integration?.repositories_synced_at + ); + }); + it('allows one Bitbucket identity to authorize personal and organization integrations', async () => { const user = await insertTestUser(); const firstOrganization = await createTestOrganization('First Org', user.id, 0); diff --git a/apps/web/src/lib/integrations/platforms/bitbucket/credentials.ts b/apps/web/src/lib/integrations/platforms/bitbucket/credentials.ts index 1f054e1d0d..0c3340b0be 100644 --- a/apps/web/src/lib/integrations/platforms/bitbucket/credentials.ts +++ b/apps/web/src/lib/integrations/platforms/bitbucket/credentials.ts @@ -8,14 +8,21 @@ import { import { db } from '@/lib/drizzle'; import { INTEGRATION_STATUS, PLATFORM } from '@/lib/integrations/core/constants'; import type { Owner } from '@/lib/integrations/core/types'; +import { + BitbucketOAuthRecoverySchema, + type BitbucketOAuthRecovery, +} from '@/lib/integrations/oauth-state'; import { ORGANIZATION_BILLING_ROLES } from '@kilocode/app-shared/organizations'; import { encryptKeyedEnvelope } from '@kilocode/encryption'; +import { getBitbucketReviewGrantStatus } from '@kilocode/worker-utils/bitbucket-workspace-access-token'; import { kilocode_users, organization_memberships, platform_integrations, platform_oauth_credentials, type NewPlatformOAuthCredential, + type PlatformIntegration, + type PlatformOAuthCredential, } from '@kilocode/db/schema'; import { and, eq, inArray, isNull, sql } from 'drizzle-orm'; import type { BitbucketOAuthTokens, BitbucketUser } from './adapter'; @@ -32,6 +39,13 @@ export class BitbucketIntegrationConnectionConflictError extends Error { } } +export class BitbucketIntegrationRecoveryError extends Error { + constructor(readonly code: 'connection_changed' | 'workspace_unavailable' | 'missing_scopes') { + super(code); + this.name = 'BitbucketIntegrationRecoveryError'; + } +} + export function buildBitbucketOAuthCredentialAad(input: { credentialId: string; integrationId: string; @@ -57,6 +71,7 @@ export type StoreBitbucketIntegrationInput = { bitbucketUser: Pick; tokens: BitbucketOAuthTokens; availableWorkspaces: BitbucketWorkspace[]; + bitbucketRecovery?: BitbucketOAuthRecovery; }; function normalizeBitbucketUuid(value: string): string { @@ -92,19 +107,98 @@ function ownerCondition(owner: Owner) { : eq(platform_integrations.owned_by_organization_id, owner.id); } +function readBitbucketOAuthRecovery( + integration: PlatformIntegration | undefined, + credential: PlatformOAuthCredential | undefined +): BitbucketOAuthRecovery { + if ( + !integration || + !credential || + integration.platform !== PLATFORM.BITBUCKET || + integration.integration_type !== 'oauth' || + integration.integration_status !== INTEGRATION_STATUS.ACTIVE || + credential.platform_integration_id !== integration.id || + credential.revoked_at + ) { + throw new BitbucketIntegrationRecoveryError('connection_changed'); + } + const metadata = BitbucketIntegrationMetadataSchema.safeParse(integration.metadata); + if ( + !metadata.success || + metadata.data.state !== 'active' || + integration.platform_account_id !== metadata.data.workspace.uuid || + integration.platform_installation_id !== metadata.data.workspace.uuid || + integration.platform_account_login !== metadata.data.workspace.slug + ) { + throw new BitbucketIntegrationRecoveryError('connection_changed'); + } + const target = BitbucketOAuthRecoverySchema.safeParse({ + integrationId: integration.id, + credentialId: credential.id, + credentialVersion: credential.credential_version, + workspaceUuid: metadata.data.workspace.uuid, + workspaceSlug: metadata.data.workspace.slug, + }); + if (!target.success) throw new BitbucketIntegrationRecoveryError('connection_changed'); + return target.data; +} + +export async function getBitbucketOAuthRecovery( + owner: Owner, + integrationId: string +): Promise { + if (!BitbucketOAuthRecoverySchema.shape.integrationId.safeParse(integrationId).success) { + throw new BitbucketIntegrationRecoveryError('connection_changed'); + } + const [current] = await db + .select({ integration: platform_integrations, credential: platform_oauth_credentials }) + .from(platform_integrations) + .innerJoin( + platform_oauth_credentials, + eq(platform_oauth_credentials.platform_integration_id, platform_integrations.id) + ) + .where( + and( + ownerCondition(owner), + eq(platform_integrations.id, integrationId), + eq(platform_integrations.platform, PLATFORM.BITBUCKET) + ) + ) + .limit(1); + return readBitbucketOAuthRecovery(current?.integration, current?.credential); +} + export async function storeBitbucketIntegration(input: StoreBitbucketIntegrationInput): Promise<{ status: 'connected' | 'workspace_selection_required'; integrationId: string; }> { - const integrationId = randomUUID(); - const credentialId = randomUUID(); + if (input.owner.type === 'user' && input.owner.id !== input.authorizedByUserId) { + throw new BitbucketIntegrationAuthorizationError( + 'Bitbucket integration authorizer is no longer authorized' + ); + } + const recovery = input.bitbucketRecovery; + if (recovery && !getBitbucketReviewGrantStatus(input.tokens.scopes, 'oauth').writeReady) { + throw new BitbucketIntegrationRecoveryError('missing_scopes'); + } + const integrationId = recovery?.integrationId ?? randomUUID(); + const credentialId = recovery?.credentialId ?? randomUUID(); const providerSubjectId = normalizeBitbucketUuid(input.bitbucketUser.uuid); const availableWorkspaces = input.availableWorkspaces.map(workspace => ({ uuid: normalizeBitbucketUuid(workspace.uuid), slug: workspace.slug, name: workspace.name, })); - const selectedWorkspace = availableWorkspaces.length === 1 ? availableWorkspaces[0] : undefined; + const selectedWorkspace = recovery + ? availableWorkspaces.find( + workspace => + workspace.uuid === recovery.workspaceUuid && workspace.slug === recovery.workspaceSlug + ) + : availableWorkspaces.length === 1 + ? availableWorkspaces[0] + : undefined; + if (recovery && !selectedWorkspace) + throw new BitbucketIntegrationRecoveryError('workspace_unavailable'); const metadata = BitbucketIntegrationMetadataSchema.parse( selectedWorkspace ? { state: 'active', workspace: selectedWorkspace } @@ -152,51 +246,100 @@ export async function storeBitbucketIntegration(input: StoreBitbucketIntegration await tx.execute( sql`SELECT pg_advisory_xact_lock(hashtextextended(${`bitbucket-oauth-owner:${input.owner.type}:${input.owner.id}`}, 0))` ); + // Use the refresh lock before row locks so a refresh cannot overwrite recovery. + if (recovery) { + await tx.execute( + sql`SELECT pg_advisory_xact_lock(hashtextextended(${`bitbucket-oauth-credential:${recovery.credentialId}`}, 0))` + ); + } - if (input.owner.type === 'org') { - const [authorizer] = await tx - .select({ isAdmin: kilocode_users.is_admin }) - .from(kilocode_users) + const [authorizer] = await tx + .select({ isAdmin: kilocode_users.is_admin }) + .from(kilocode_users) + .where( + and(eq(kilocode_users.id, input.authorizedByUserId), isNull(kilocode_users.blocked_reason)) + ) + .for('update'); + if (!authorizer) { + throw new BitbucketIntegrationAuthorizationError( + 'Bitbucket integration authorizer is no longer authorized' + ); + } + if (input.owner.type === 'org' && !authorizer.isAdmin) { + const [membership] = await tx + .select({ id: organization_memberships.id }) + .from(organization_memberships) .where( and( - eq(kilocode_users.id, input.authorizedByUserId), - isNull(kilocode_users.blocked_reason) + eq(organization_memberships.organization_id, input.owner.id), + eq(organization_memberships.kilo_user_id, input.authorizedByUserId), + inArray(organization_memberships.role, ORGANIZATION_BILLING_ROLES) ) ) .for('update'); - if (!authorizer) { + if (!membership) { throw new BitbucketIntegrationAuthorizationError( 'Bitbucket integration authorizer is no longer authorized' ); } - - if (!authorizer.isAdmin) { - const [membership] = await tx - .select({ id: organization_memberships.id }) - .from(organization_memberships) - .where( - and( - eq(organization_memberships.organization_id, input.owner.id), - eq(organization_memberships.kilo_user_id, input.authorizedByUserId), - inArray(organization_memberships.role, ORGANIZATION_BILLING_ROLES) - ) - ) - .for('update'); - if (!membership) { - throw new BitbucketIntegrationAuthorizationError( - 'Bitbucket integration authorizer is no longer authorized' - ); - } - } } const [currentIntegration] = await tx - .select({ id: platform_integrations.id }) + .select() .from(platform_integrations) .where( and(ownerCondition(input.owner), eq(platform_integrations.platform, PLATFORM.BITBUCKET)) ) .for('update'); + if (recovery) { + if (!currentIntegration || currentIntegration.id !== integrationId) { + throw new BitbucketIntegrationRecoveryError('connection_changed'); + } + const [currentCredential] = await tx + .select() + .from(platform_oauth_credentials) + .where(eq(platform_oauth_credentials.platform_integration_id, integrationId)) + .for('update'); + const current = readBitbucketOAuthRecovery(currentIntegration, currentCredential); + if ( + current.integrationId !== recovery.integrationId || + current.credentialId !== recovery.credentialId || + current.credentialVersion !== recovery.credentialVersion || + current.workspaceUuid !== recovery.workspaceUuid || + current.workspaceSlug !== recovery.workspaceSlug + ) { + throw new BitbucketIntegrationRecoveryError('connection_changed'); + } + + const updatedAt = new Date().toISOString(); + const [updatedCredential] = await tx + .update(platform_oauth_credentials) + .set({ + ...credentialValues, + credential_version: current.credentialVersion + 1, + updated_at: updatedAt, + }) + .where( + and( + eq(platform_oauth_credentials.id, current.credentialId), + eq(platform_oauth_credentials.platform_integration_id, integrationId), + eq(platform_oauth_credentials.credential_version, current.credentialVersion) + ) + ) + .returning({ id: platform_oauth_credentials.id }); + if (!updatedCredential) throw new BitbucketIntegrationRecoveryError('connection_changed'); + + const [updatedIntegration] = await tx + .update(platform_integrations) + .set({ + scopes: [...input.tokens.scopes], + updated_at: updatedAt, + }) + .where(and(ownerCondition(input.owner), eq(platform_integrations.id, integrationId))) + .returning({ id: platform_integrations.id }); + if (!updatedIntegration) throw new BitbucketIntegrationRecoveryError('connection_changed'); + return { status: 'connected', integrationId }; + } if (currentIntegration) { throw new BitbucketIntegrationConnectionConflictError(); } diff --git a/apps/web/src/lib/integrations/platforms/bitbucket/workspace-access-token-repository-cache.ts b/apps/web/src/lib/integrations/platforms/bitbucket/workspace-access-token-repository-cache.ts index 42b3be3180..8caac54159 100644 --- a/apps/web/src/lib/integrations/platforms/bitbucket/workspace-access-token-repository-cache.ts +++ b/apps/web/src/lib/integrations/platforms/bitbucket/workspace-access-token-repository-cache.ts @@ -7,6 +7,7 @@ import { BITBUCKET_WORKSPACE_ACCESS_TOKEN_REQUIRED_EFFECTIVE_SCOPES, BitbucketWorkspaceAccessTokenCredentialRowSchema, getMissingBitbucketWorkspaceAccessTokenScopes, + getBitbucketReviewGrantStatus, getUnexpectedBitbucketWorkspaceAccessTokenScopes, hasRequiredBitbucketWorkspaceAccessTokenScopes, } from '@kilocode/worker-utils/bitbucket-workspace-access-token'; @@ -314,7 +315,21 @@ export async function getBitbucketWorkspaceAccessTokenStatus(organizationId: str if (!integration) return notConnectedStatus(); const invalidationReason = InvalidationReasonSchema.safeParse(integration.row.authInvalidReason); + // Old optimistic status producers omit this addition until refetch. Remove only after old + // clients/records disappear and the 30-day ledger window expires; absence never proves write access. + const permissions: { + reviewPermissions?: ReturnType | null; + } = { + reviewPermissions: + integration.state === 'usable' + ? getBitbucketReviewGrantStatus( + integration.credentialProfile?.provider_scopes ?? [], + 'workspace_access_token' + ) + : null, + }; const statusDetails = { + ...permissions, method: BITBUCKET_WORKSPACE_ACCESS_TOKEN_INTEGRATION_TYPE, integrationId: integration.row.integrationId, integrationStatus: integration.row.integrationStatus, diff --git a/apps/web/src/lib/provider-review/bitbucket-write.test.ts b/apps/web/src/lib/provider-review/bitbucket-write.test.ts new file mode 100644 index 0000000000..4e225373dc --- /dev/null +++ b/apps/web/src/lib/provider-review/bitbucket-write.test.ts @@ -0,0 +1,1316 @@ +jest.mock('@/lib/drizzle', () => ({ db: { select: jest.fn() } })); +jest.mock('@/lib/config.server', () => ({})); +jest.mock('./bitbucket-read', () => ({ + getBitbucketReview: jest.fn(), + listBitbucketFiles: jest.fn(), +})); +jest.mock('./operation', () => ({ + ...jest.requireActual('./operation'), + runReviewOperation: jest.fn(), +})); +jest.mock( + '@/lib/integrations/platforms/bitbucket/workspace-access-token-organization-authorization', + () => ({}) +); + +import { db } from '@/lib/drizzle'; +import { repositoryResourceKey } from '@kilocode/app-shared/code-review/repository-identity'; +import { + providerReviewIntentFingerprint, + type BitbucketMergeEvidence, + type ReviewIntentInput, + type ReviewOverview, + type ReviewPosition, +} from '@kilocode/app-shared/provider-review'; +import { reviewCapabilityFixtures } from '@kilocode/app-shared/provider-review/fixtures'; +import { BitbucketInteractiveClientError } from '@/lib/integrations/platforms/bitbucket/interactive-client'; +import { getBitbucketWorkspaceAccessTokenStatus } from '@/lib/integrations/platforms/bitbucket/workspace-access-token-repository-cache'; +import { createBitbucketInteractiveApi } from '../../../../../services/git-token-service/src/bitbucket-interactive-api'; +import { + BitbucketApiError, + BitbucketInteractiveError, +} from '../../../../../services/git-token-service/src/bitbucket-safe-transport'; +import type { BitbucketReviewAuthorization } from './bitbucket-authorization'; +import { getBitbucketReview, listBitbucketFiles } from './bitbucket-read'; +import { + rejectedReviewEffect, + unresolvedReviewEffect, + reviewEffectOperationKey, + runReviewOperation, + type ReviewEffectResult, +} from './operation'; +import { + runBitbucketReviewOperation, + type BitbucketReviewOperationRequest as ReviewOperationRequest, +} from './bitbucket-write'; + +const userId = 'oauth/caller'; +const authorization = { + kind: 'ownerIntegration' as const, + owner: { type: 'org' as const, id: '11111111-1111-4111-8111-111111111111' }, + integrationId: '22222222-2222-4222-8222-222222222222', +}; +const repository = { + provider: 'bitbucket' as const, + instanceUrl: 'https://bitbucket.org', + workspaceUuid: '33333333-3333-4333-8333-333333333333', + repositoryId: '44444444-4444-4444-8444-444444444444', + fullName: 'team/repo', + defaultBranch: 'trunk', +}; +const actorId = '55555555-5555-4555-8555-555555555555'; +const identity = { + repository, + authorization, + number: '7', + reviewId: '7', + canonicalUrl: 'https://bitbucket.org/team/repo/pull-requests/7', +}; +const revision = { + headSha: 'a'.repeat(40), + targetHeadSha: 'b'.repeat(40), + baseSha: null, + startSha: null, +}; +const position: ReviewPosition = { + revision, + oldPath: 'old.ts', + newPath: 'new.ts', + side: 'new', + line: 4, + startSide: 'new', + startLine: 2, + native: { provider: 'bitbucket', to: 4, startTo: 2 }, +}; +const nativeRepo = { + uuid: `{${repository.repositoryId}}`, + full_name: repository.fullName, + workspace: { uuid: `{${repository.workspaceUuid}}` }, +}; +const root = `/2.0/repositories/{${repository.workspaceUuid}}/{${repository.repositoryId}}/pullrequests/7`; +const taskUrl = `https://api.bitbucket.org${root.replaceAll('{', '%7B').replaceAll('}', '%7D')}/merge/task-status/task-1`; +const target = { + provider: 'bitbucket' as const, + kind: 'thread' as const, + id: '1', + url: `${identity.canonicalUrl}/_/diff#comment-1`, +}; +let auth: BitbucketReviewAuthorization; +let overview: ReviewOverview; +let pr: any; +let comments: any[]; +let state: 'approved' | 'changes_requested' | null; +let merges: any[]; +let events: string[]; +let afterWrite: (() => void) | undefined; +let lostResponse: boolean; +let pendingMerge: boolean; +let taskState: 'PENDING' | 'SUCCESS' | 'error' | 'missing'; +let taskSelf: string; +let taskLocation: string; +let responseOverride: ((value: any) => any) | undefined; +let readFailure: boolean; +let branchExists: boolean; +let sourceCommits: string[]; +let sourceInDestination: boolean; +type StoredEffect = { + fingerprint: string; + result: ReviewEffectResult; + active: boolean; + mergeEvidence?: BitbucketMergeEvidence; +}; +let records: Map; +const request = (input: ReviewIntentInput): ReviewOperationRequest => ({ + userId, + distinctId: 'caller', + operationKey: '66666666-6666-4666-8666-666666666666', + intent: { accountId: userId, actorId: auth.actor.id, review: identity, revision, input }, +}); +const run = (input: ReviewIntentInput, statusOnly = false) => + runBitbucketReviewOperation(auth, request(input), statusOnly); +function finishMerge() { + pr.state = 'MERGED'; + pr.merge_commit = { hash: 'c'.repeat(40) }; +} + +beforeEach(() => { + jest.resetAllMocks(); + records = new Map(); + comments = [ + { + type: 'pullrequest_comment', + id: 1, + content: { raw: 'Original' }, + pullrequest: { id: 7 }, + user: { uuid: actorId }, + }, + ]; + pr = { + type: 'pullrequest', + id: 7, + state: 'OPEN', + links: { html: { href: identity.canonicalUrl } }, + source: { + repository: nativeRepo, + branch: { name: 'feature' }, + commit: { hash: revision.headSha }, + }, + destination: { + repository: nativeRepo, + branch: { name: 'trunk' }, + commit: { hash: revision.targetHeadSha }, + }, + }; + state = null; + merges = []; + events = []; + afterWrite = undefined; + lostResponse = false; + pendingMerge = false; + taskState = 'PENDING'; + taskSelf = taskUrl; + taskLocation = taskUrl; + responseOverride = undefined; + readFailure = false; + branchExists = true; + sourceCommits = [revision.headSha]; + sourceInDestination = true; + const api = createBitbucketInteractiveApi({ + scope: { + kind: 'repository', + workspace: `{${repository.workspaceUuid}}`, + repository: `{${repository.repositoryId}}`, + }, + accessToken: 'provider-fixture', + // Mirror the broker's verified repository aliases; request paths remain UUID-based. + canonicalTaskRepository: { + workspace: repository.fullName.split('/')[0], + repository: repository.fullName.split('/')[1], + }, + fetch: async (url, init) => { + const route = decodeURIComponent(new URL(String(url)).pathname); + const method = init?.method; + if (new Headers(init?.headers).has('if-match')) throw new Error('Undocumented guard'); + if (method === 'GET') { + if (route === `${root}/commits`) + return Response.json({ values: sourceCommits.map(hash => ({ hash })) }); + if ( + !sourceInDestination && + route.includes('/commit/') && + revision.headSha.startsWith(route.split('/').at(-1)!) + ) + return Response.json({}, { status: 404 }); + if (route === root) + return Response.json(pr, { status: readFailure && events.length ? 503 : 200 }); + if (route === `${root}/merge/task-status/task-1`) { + if (taskState === 'error') + return Response.json({ type: 'error', error: { message: 'Merge failed' } }); + if (taskState === 'SUCCESS') finishMerge(); + return Response.json({ + task_status: taskState === 'missing' ? 'SUCCESS' : taskState, + links: { self: { href: taskSelf } }, + ...(taskState === 'SUCCESS' + ? { merge_result: responseOverride ? responseOverride(pr) : pr } + : {}), + }); + } + if (route.startsWith(`${root}/comments/`)) { + const value = comments.find(comment => String(comment.id) === route.split('/').at(-1)); + return Response.json(value ?? {}, { status: value ? 200 : 404 }); + } + if (route.endsWith('/refs/branches/feature')) + return Response.json( + { name: 'feature', target: { hash: revision.headSha } }, + { status: branchExists ? 200 : 404 } + ); + if (route.includes('/commit/')) { + const short = route.split('/').at(-1)!; + const hash = [revision.headSha, revision.targetHeadSha, 'c'.repeat(40)].find(value => + value.startsWith(short) + ); + return Response.json({ hash }, { status: hash ? 200 : 404 }); + } + return Response.json({}, { status: 404 }); + } + let value: unknown = null, + status = method === 'DELETE' ? 204 : 200; + const body = init?.body ? JSON.parse(String(init.body)) : {}; + if (route === `${root}/comments` && method === 'POST') { + value = { + ...body, + id: comments.length + 1, + pullrequest: { id: 7 }, + user: { uuid: actorId }, + }; + comments.push(value); + status = 201; + events.push('comment'); + } else if (route === `${root}/comments/1/resolve`) { + comments[0].resolution = method === 'POST' ? { type: 'comment_resolution' } : null; + value = comments[0].resolution; + events.push(method === 'POST' ? 'resolve' : 'reopen'); + } else if (route === `${root}/approve` || route === `${root}/request-changes`) { + state = + method === 'DELETE' + ? null + : route.endsWith('/approve') + ? 'approved' + : 'changes_requested'; + value = + method === 'DELETE' ? null : { type: 'participant', user: { uuid: actorId }, state }; + events.push(`${method}:${route.split('/').at(-1)}`); + } else if (route === `${root}/merge` && method === 'POST') { + merges.push(body); + events.push('merge'); + if (!pendingMerge) finishMerge(); + value = pr; + } else return Response.json({}, { status: 400 }); + afterWrite?.(); + if (lostResponse) throw new Error('Provider committed; response lost'); + if (pendingMerge && route.endsWith('/merge')) + return new Response(null, { status: 202, headers: { location: taskLocation } }); + return status === 204 + ? new Response(null, { status }) + : Response.json(responseOverride ? responseOverride(value) : value, { status }); + }, + }); + auth = { + userId, + authorization, + repository, + path: { workspace: `{${repository.workspaceUuid}}`, repo_slug: `{${repository.repositoryId}}` }, + actor: { + provider: 'bitbucket', + instanceUrl: repository.instanceUrl, + id: actorId, + login: 'service-actor', + displayName: null, + avatarUrl: null, + }, + credentialKind: 'bitbucketOAuth', + scopes: [ + 'account', + 'repository', + 'repository:write', + 'pullrequest', + 'pullrequest:write', + 'webhook', + ], + client: { + execute: async input => { + try { + return { ...(await api.execute(input)), metadata: {} } as any; + } catch (error) { + if (error instanceof BitbucketApiError || error instanceof BitbucketInteractiveError) + throw new BitbucketInteractiveClientError(error.code); + throw error; + } + }, + }, + }; + overview = { + identity, + title: 'Review', + bodyMarkdown: null, + author: null, + state: 'open', + draft: false, + revision, + source: { repository, branch: 'feature' }, + target: { repository, branch: 'trunk' }, + authorization: { + actor: auth.actor, + credentialKind: auth.credentialKind, + capabilities: reviewCapabilityFixtures('bitbucket'), + writeLimits: { requestMaxBytes: 256_000, bodyMaxBytes: null }, + }, + providerState: { provider: 'bitbucket', expectedHeadProtection: 'none', participants: [] }, + checks: { status: 'none', checks: [] }, + counts: { files: 1, commits: 1, additions: 1, deletions: 0 }, + merge: { + methods: [ + 'merge_commit', + 'squash', + 'fast_forward', + 'squash_fast_forward', + 'rebase_fast_forward', + 'rebase_merge', + ].map(id => ({ id, label: id })), + squash: null, + autoMerge: null, + task: null, + }, + }; + jest.mocked(getBitbucketReview).mockImplementation(async () => structuredClone(overview)); + jest.mocked(listBitbucketFiles).mockResolvedValue({ + items: [ + { + id: 'file', + oldPath: 'old.ts', + newPath: 'new.ts', + revision, + status: 'renamed', + patch: null, + content: 'available', + additions: 1, + deletions: 0, + canonicalUrl: null, + }, + ], + nextCursor: null, + }); + // The c3 suite tests PostgreSQL admission. This stateful boundary makes a missing ledger call + // observable here as a duplicate provider effect, including after status-only recovery. + jest.mocked(runReviewOperation).mockImplementation(async (input, handlers) => { + const key = reviewEffectOperationKey(input.operationKey, input.effect?.id); + const fingerprint = providerReviewIntentFingerprint(input.intent); + const existing = records.get(key); + if (existing?.fingerprint && existing.fingerprint !== fingerprint) + return rejectedReviewEffect('operation_key_reuse_mismatch'); + if (existing?.active) return unresolvedReviewEffect('operation_in_progress'); + if ( + existing && + (existing.result.status === 'confirmed' || + (existing.result.status === 'rejected' && existing.result.retry === 'never')) + ) + return existing.result; + if (!existing && !handlers.execute) + return rejectedReviewEffect('operation_not_admitted', 'same-key'); + const row: StoredEffect = existing ?? { + fingerprint, + result: unresolvedReviewEffect('dispatching'), + active: false, + }; + records.set(key, row); + row.active = true; + const stored = existing?.result ?? null; + const result = await (handlers.execute && + (!existing || (stored?.status === 'rejected' && stored.retry === 'same-key')) + ? handlers.execute(async evidence => { + row.mergeEvidence = structuredClone(evidence); + }) + : handlers.reconcile(stored, row.mergeEvidence)); + row.active = false; + if (!(result.status === 'unresolved' && stored?.status === 'accepted')) row.result = result; + return result; + }); +}); + +it.each([ + { action: 'comment', body: 'Summary' }, + { action: 'inlineComment', body: 'Inline', position }, + { action: 'reply', body: 'Reply', target }, +] satisfies ReviewIntentInput[])( + 'AC6 posts $action to the PR and never claims atomic revision success', + async input => { + const result = await run(input); + expect(result).toMatchObject({ + status: 'unresolved', + reason: 'no_atomic_revision_guard', + reference: { kind: 'comment', id: '2' }, + retry: 'reconcile', + }); + expect(comments[1]).toMatchObject({ + content: { raw: input.body }, + pullrequest: { id: 7 }, + user: { uuid: actorId }, + }); + if (input.action === 'inlineComment') + expect(comments[1].inline).toEqual({ path: 'new.ts', to: 4, start_to: 2 }); + if (input.action === 'reply') expect(comments[1].parent.id).toBe(1); + expect(await run(input, true)).toEqual(result); + await run(input); + expect(comments).toHaveLength(2); + } +); + +it('AC6 preserves old-side path and range rather than translating them to the new side', async () => { + const old: ReviewPosition = { + ...position, + side: 'old', + startSide: 'old', + native: { provider: 'bitbucket', from: 4, startFrom: 2 }, + }; + await run({ action: 'inlineComment', body: 'Old range', position: old }); + expect(comments[1].inline).toEqual({ path: 'old.ts', from: 4, start_from: 2 }); +}); + +it.each(['approve', 'unapprove', 'requestChanges', 'removeChangeRequest'] as const)( + 'AC6 applies %s under the authorized actor', + async action => { + state = + action === 'unapprove' + ? 'approved' + : action === 'removeChangeRequest' + ? 'changes_requested' + : null; + expect(await run({ action })).toMatchObject({ + status: 'unresolved', + reason: 'no_atomic_revision_guard', + }); + expect(state).toBe( + action === 'approve' ? 'approved' : action === 'requestChanges' ? 'changes_requested' : null + ); + await run({ action }); + expect(events).toHaveLength(1); + } +); + +it.each(['resolveThread', 'reopenThread'] as const)( + 'AC6 applies %s to the exact PR comment', + async action => { + comments[0].resolution = action === 'reopenThread' ? {} : null; + expect(await run({ action, target })).toMatchObject({ + status: 'unresolved', + reason: 'no_atomic_revision_guard', + }); + expect(comments[0].resolution != null).toBe(action === 'resolveThread'); + await run({ action, target }); + expect(events).toHaveLength(1); + } +); + +it.each(['headSha', 'targetHeadSha'] as const)( + 'AC6 rejects stale %s before a provider write', + async field => { + overview.revision = { ...revision, [field]: 'd'.repeat(40) }; + expect(await run({ action: 'comment', body: 'Draft' })).toMatchObject({ + status: 'rejected', + code: 'conflict', + retry: 'never', + }); + expect(comments).toHaveLength(1); + } +); + +it.each(['head', 'target', 'branch', 'resource', 'read failure'] as const)( + 'AC6 retains uncertainty after postflight %s drift', + async change => { + afterWrite = () => { + if (change === 'head') pr.source.commit.hash = 'd'.repeat(40); + if (change === 'target') pr.destination.commit.hash = 'd'.repeat(40); + if (change === 'branch') pr.source.branch.name = 'different'; + if (change === 'resource') pr.id = 8; + if (change === 'read failure') readFailure = true; + }; + expect(await run({ action: 'comment', body: 'Draft' })).toMatchObject({ + status: 'unresolved', + reason: 'provider_outcome_unknown', + reference: { id: '2' }, + }); + await run({ action: 'comment', body: 'Draft' }); + expect(comments).toHaveLength(2); + } +); + +it.each(['id', 'body', 'position', 'actor'] as const)( + 'AC6 rejects incorrect returned %s evidence', + async field => { + responseOverride = value => ({ + ...value, + ...(field === 'id' + ? { pullrequest: { id: 8 } } + : field === 'body' + ? { content: { raw: 'Other' } } + : field === 'position' + ? { inline: { path: 'other.ts', to: 4, start_to: 2 } } + : { user: { uuid: '77777777-7777-4777-8777-777777777777' } }), + }); + expect(await run({ action: 'inlineComment', body: 'Draft', position })).toMatchObject({ + status: 'unresolved', + reason: 'provider_outcome_unknown', + reference: null, + }); + expect(comments).toHaveLength(2); + } +); + +it.each(['comment', 'reply', 'approve', 'resolveThread'] as const)( + 'AC6 never repeats %s after a lost response', + async action => { + const input: ReviewIntentInput = + action === 'comment' + ? { action, body: 'Draft' } + : action === 'reply' + ? { action, body: 'Reply', target } + : action === 'resolveThread' + ? { action, target } + : { action }; + lostResponse = true; + expect(await run(input)).toMatchObject({ status: 'unresolved', retry: 'reconcile' }); + lostResponse = false; + await run(input, true); + await run(input); + expect(events).toHaveLength(1); + } +); + +it('AC6 keeps each batch receipt and does not duplicate summary, inline, or decision effects', async () => { + const input: ReviewIntentInput = { + action: 'submitReview', + body: 'Summary', + choice: 'approve', + comments: [{ itemId: 'one', body: 'Inline', position }], + }; + const result = await run(input); + expect(result).toMatchObject({ + status: 'partial', + items: [ + { itemId: 'comment:one', result: { status: 'unresolved', reference: { id: '2' } } }, + { itemId: 'summary', result: { status: 'unresolved', reference: { id: '3' } } }, + { itemId: 'decision', result: { status: 'unresolved' } }, + ], + }); + expect(comments.map(value => value.content.raw)).toEqual(['Original', 'Inline', 'Summary']); + expect(state).toBe('approved'); + await run(input, true); + await run(input); + expect(comments).toHaveLength(3); + expect(events).toHaveLength(3); + expect(records.size).toBe(4); +}); + +it('AC6 stops an uncertain batch without discarding unfinished items', async () => { + lostResponse = true; + expect( + await run({ + action: 'submitReview', + body: 'Summary', + comments: [{ itemId: 'one', body: 'Inline', position }], + }) + ).toMatchObject({ + status: 'partial', + items: [ + { itemId: 'comment:one', result: { status: 'unresolved' } }, + { itemId: 'summary', result: { status: 'rejected', retry: 'same-key' } }, + ], + }); + expect(comments.map(value => value.content.raw)).toEqual(['Original', 'Inline']); +}); + +it('AC6 admits an empty review without fabricating a provider effect', async () => { + expect(await run({ action: 'submitReview', choice: 'comment', comments: [] })).toMatchObject({ + status: 'confirmed', + reference: null, + }); + expect(events).toEqual([]); + expect(comments).toHaveLength(1); +}); + +it.each([ + 'merge_commit', + 'squash', + 'fast_forward', + 'squash_fast_forward', + 'rebase_fast_forward', + 'rebase_merge', +])('AC7 uses the supported %s destination strategy', async method => { + expect(await run({ action: 'merge', method })).toMatchObject({ + status: 'confirmed', + reference: { kind: 'review', id: '7' }, + }); + expect(pr.state).toBe('MERGED'); + expect(merges).toEqual([ + { type: 'pullrequest_merge_parameters', merge_strategy: method, close_source_branch: false }, + ]); + await run({ action: 'merge', method }); + expect(merges).toHaveLength(1); +}); + +it.each(['empty methods', 'restriction', 'permission'] as const)( + 'AC7 blocks merge for %s', + async condition => { + if (condition === 'empty methods') overview.merge.methods = []; + if (condition === 'restriction') + overview.authorization.capabilities.merge.restrictions = ['changes_requested']; + if (condition === 'permission') + overview.authorization.capabilities.merge.permission = 'forbidden'; + expect(await run({ action: 'merge', method: 'merge_commit' })).toMatchObject({ + status: 'rejected', + retry: 'never', + }); + expect(merges).toEqual([]); + expect(pr.state).toBe('OPEN'); + } +); + +it('AC7 persists and polls the same accepted task before confirming its merge', async () => { + pendingMerge = true; + const input: ReviewIntentInput = { action: 'merge', method: 'merge_commit' }; + expect(await run(input)).toMatchObject({ + status: 'accepted', + reference: { id: 'task-1', url: taskUrl }, + task: { state: 'pending' }, + }); + expect([...records.values()][0].result).toMatchObject({ + status: 'accepted', + reference: { id: 'task-1' }, + }); + expect(await run(input, true)).toMatchObject({ status: 'accepted' }); + taskState = 'SUCCESS'; + expect(await run(input, true)).toMatchObject({ status: 'confirmed' }); + expect(pr.state).toBe('MERGED'); + expect(merges).toHaveLength(1); +}); + +it.each(['error', 'missing', 'wrong task', 'wrong review', 'drift'] as const)( + 'AC7 keeps %s task evidence unresolved without resubmitting', + async condition => { + pendingMerge = true; + const input: ReviewIntentInput = { action: 'merge', method: 'merge_commit' }; + await run(input); + taskState = condition === 'error' || condition === 'missing' ? condition : 'SUCCESS'; + if (condition === 'wrong task') taskSelf = taskUrl.replace('task-1', 'task-2'); + if (condition === 'wrong review') responseOverride = value => ({ ...value, id: 8 }); + if (condition === 'drift') pr.source.commit.hash = 'd'.repeat(40); + expect(await run(input, true)).toMatchObject({ status: 'unresolved', retry: 'reconcile' }); + expect([...records.values()][0].result).toMatchObject({ + status: 'accepted', + reference: { id: 'task-1' }, + }); + await run(input); + expect(merges).toHaveLength(1); + } +); + +it('AC7 reconciles a lost merge response without another merge', async () => { + lostResponse = true; + const input: ReviewIntentInput = { action: 'merge', method: 'merge_commit' }; + expect(await run(input)).toMatchObject({ status: 'unresolved' }); + lostResponse = false; + expect(await run(input, true)).toMatchObject({ status: 'confirmed' }); + expect(merges).toHaveLength(1); +}); + +it.each([true, false])( + 'AC7 reports branch deletion separately when the branch remains: %s', + async exists => { + afterWrite = () => { + branchExists = exists; + }; + const input: ReviewIntentInput = { + action: 'merge', + method: 'merge_commit', + deletion: { + effect: 'delete', + branch: 'feature', + expectedHeadSha: revision.headSha, + repositoryKey: repositoryResourceKey(userId, { repository, authorization }), + }, + }; + const result = await run(input); + expect(result).toMatchObject( + exists + ? { + status: 'partial', + items: [ + { effect: 'merge', result: { status: 'confirmed' } }, + { + effect: 'deleteBranch', + result: { status: 'unresolved', reason: 'source_branch_still_present' }, + }, + ], + } + : { status: 'confirmed' } + ); + expect(merges[0].close_source_branch).toBe(true); + await run(input, true); + expect(events).toEqual(['merge']); + expect(records.size).toBe(3); + } +); + +it('AC7 does not infer deletion from a masked access failure', async () => { + afterWrite = () => { + branchExists = false; + }; + const execute = auth.client.execute; + auth.client.execute = async input => { + try { + return await execute(input); + } catch (error) { + if (input.operation === 'branch') readFailure = true; + throw error; + } + }; + expect( + await run({ + action: 'merge', + method: 'merge_commit', + deletion: { + effect: 'delete', + branch: 'feature', + expectedHeadSha: revision.headSha, + repositoryKey: repositoryResourceKey(userId, { repository, authorization }), + }, + }) + ).toMatchObject({ + status: 'partial', + items: [ + { effect: 'merge', result: { status: 'confirmed' } }, + { + effect: 'deleteBranch', + result: { status: 'unresolved', reason: 'source_branch_deletion_unknown' }, + }, + ], + }); + expect(events).toEqual(['merge']); +}); + +it('AC1 keeps read-only grants usable and requires a new admitted action after reconnect', async () => { + overview.authorization.capabilities.approve.permission = 'forbidden'; + expect(await run({ action: 'approve' })).toMatchObject({ + status: 'rejected', + code: 'insufficient_permissions', + }); + expect(events).toEqual([]); + overview.authorization.capabilities.approve.permission = 'allowed'; + const replacement = request({ action: 'approve' }); + replacement.operationKey = '88888888-8888-4888-8888-888888888888'; + expect(await runBitbucketReviewOperation(auth, replacement)).toMatchObject({ + status: 'unresolved', + reason: 'no_atomic_revision_guard', + }); + expect(state).toBe('approved'); +}); + +it('AC6 distinguishes retryable preflight failure from an uncertain write', async () => { + jest + .mocked(getBitbucketReview) + .mockRejectedValueOnce(new BitbucketInteractiveClientError('temporarily_unavailable')); + expect(await run({ action: 'comment', body: 'Draft' })).toMatchObject({ + status: 'rejected', + retry: 'same-key', + }); + expect(comments).toHaveLength(1); + expect(await run({ action: 'comment', body: 'Draft' })).toMatchObject({ + status: 'unresolved', + retry: 'reconcile', + }); + expect(comments).toHaveLength(2); +}); + +it('AC6 rejects missing revisions, wrong actors, malformed positions, and wrong target resources', async () => { + const missing = request({ action: 'approve' }); + delete (missing.intent as any).revision; + await expect(runBitbucketReviewOperation(auth, missing)).rejects.toMatchObject({ + code: 'invalid_request', + }); + const other = request({ action: 'approve' }); + other.intent.actorId = 'other'; + expect(await runBitbucketReviewOperation(auth, other)).toMatchObject({ + status: 'rejected', + code: 'operation_identity_mismatch', + }); + await expect( + run({ action: 'inlineComment', body: 'Draft', position: { ...position, line: 5 } }) + ).rejects.toMatchObject({ code: 'conflict' }); + await expect( + run({ + action: 'reply', + body: 'Reply', + target: { ...target, url: target.url.replace('/7/', '/8/') }, + }) + ).rejects.toMatchObject({ code: 'invalid_request' }); + expect(events).toEqual([]); +}); + +it('AC6 binds a file base independently from the overview revision', async () => { + const fileRevision = { ...revision, baseSha: 'd'.repeat(40) }; + jest.mocked(listBitbucketFiles).mockResolvedValueOnce({ + items: [{ oldPath: 'old.ts', newPath: 'new.ts', revision: fileRevision } as any], + nextCursor: null, + }); + expect( + await run({ + action: 'inlineComment', + body: 'File base', + position: { ...position, revision: fileRevision }, + }) + ).toMatchObject({ status: 'unresolved', reference: { id: '2' } }); + expect(comments[1].inline).toEqual({ path: 'new.ts', to: 4, start_to: 2 }); +}); + +it('AC6 never upgrades an unverified participant receipt during status recovery', async () => { + lostResponse = true; + await run({ action: 'approve' }); + lostResponse = false; + expect(await run({ action: 'approve' }, true)).toMatchObject({ + status: 'unresolved', + reason: 'provider_outcome_unknown', + }); + expect(events).toEqual(['POST:approve']); +}); + +it('AC6 keeps a post-write credential fence unresolved rather than reporting rejection', async () => { + const execute = auth.client.execute; + auth.client.execute = async input => { + const result = await execute(input); + if (input.operation === 'approve') + throw new BitbucketInteractiveClientError('reconnect_required'); + return result; + }; + expect(await run({ action: 'approve' })).toMatchObject({ + status: 'unresolved', + retry: 'reconcile', + }); + expect(state).toBe('approved'); + await run({ action: 'approve' }, true); + await run({ action: 'approve' }); + expect(events).toEqual(['POST:approve']); +}); + +it.each(['approve', 'unapprove', 'requestChanges', 'removeChangeRequest', 'merge'] as const)( + 'AC6/AC7 rejects %s when the final preflight finds a closed PR', + async action => { + pr.state = 'DECLINED'; + expect( + await run(action === 'merge' ? { action, method: 'merge_commit' } : { action }) + ).toMatchObject({ status: 'rejected', code: 'conflict' }); + expect(events).toEqual([]); + } +); + +it('AC6 admits concurrent taps once before the provider responds', async () => { + const gate = Promise.withResolvers(); + jest.mocked(getBitbucketReview).mockImplementationOnce(async () => { + await gate.promise; + return overview; + }); + const first = run({ action: 'comment', body: 'Once' }); + await Promise.resolve(); + expect(await run({ action: 'comment', body: 'Once' })).toMatchObject({ + status: 'unresolved', + reason: 'operation_in_progress', + }); + gate.resolve(); + await first; + expect(comments.map(value => value.content.raw)).toEqual(['Original', 'Once']); +}); + +it.each(['missing merge commit', 'changed head', 'changed destination'] as const)( + 'AC7 leaves %s evidence unresolved even after a successful response', + async defect => { + afterWrite = () => { + if (defect === 'missing merge commit') delete pr.merge_commit; + if (defect === 'changed head') pr.source.commit.hash = 'd'.repeat(40); + if (defect === 'changed destination') pr.destination.commit.hash = 'd'.repeat(40); + }; + expect(await run({ action: 'merge', method: 'merge_commit' })).toMatchObject({ + status: 'unresolved', + retry: 'reconcile', + }); + await run({ action: 'merge', method: 'merge_commit' }); + expect(merges).toHaveLength(1); + } +); + +it.each(['branch', 'repository', 'head', 'protected source', 'fork'] as const)( + 'AC7 rejects an unsafe deletion %s before merging', + async defect => { + const deletion = { + effect: 'delete' as const, + branch: 'feature', + expectedHeadSha: revision.headSha, + repositoryKey: repositoryResourceKey(userId, { repository, authorization }), + }; + if (defect === 'branch') deletion.branch = 'trunk'; + if (defect === 'repository') deletion.repositoryKey = 'other-repository'; + if (defect === 'head') deletion.expectedHeadSha = 'd'.repeat(40); + if (defect === 'protected source') + overview.authorization.capabilities.deleteBranch.restrictions = ['source_branch_protected']; + if (defect === 'fork') + overview.source.repository = { + ...repository, + repositoryId: '77777777-7777-4777-8777-777777777777', + }; + expect(await run({ action: 'merge', method: 'merge_commit', deletion })).toMatchObject({ + status: 'partial', + items: [ + { effect: 'merge', result: { status: 'rejected', code: 'conflict' } }, + { effect: 'deleteBranch', result: { status: 'unresolved' } }, + ], + }); + expect(merges).toEqual([]); + expect(events).toEqual([]); + } +); + +it.each([ + ['legacy read', ['account', 'repository:write', 'pullrequest', 'webhook']], + ['write implies read', ['account', 'pullrequest:write', 'webhook']], +] as const)( + 'AC1/AC6 permits comments with %s grants through the real c2 preflight', + async (_label, scopes) => { + auth.scopes = [...scopes]; + auth.credentialKind = 'bitbucketWorkspaceToken'; + auth.actor = { ...auth.actor, id: `workspace:${repository.workspaceUuid}` }; + Object.assign(pr, { title: 'Review', updated_on: '2026-08-30T00:00:00Z', participants: [] }); + const providerRepository = { + ...nativeRepo, + workspace: { ...nativeRepo.workspace, slug: 'team' }, + }; + pr.source.repository = providerRepository; + pr.destination.repository = providerRepository; + const execute = auth.client.execute; + auth.client.execute = async input => { + switch (input.operation) { + case 'diffstat': + case 'statuses': + case 'restrictions': + return { status: 200, data: { values: [] }, metadata: {} } as any; + case 'commits': + return { + status: 200, + data: { values: [{ hash: revision.headSha }] }, + metadata: {}, + } as any; + case 'branch': + return { + status: 200, + data: { name: 'trunk', merge_strategies: ['merge_commit'] }, + metadata: {}, + } as any; + default: + return execute(input); + } + }; + const actual = jest.requireActual<{ getBitbucketReview: typeof getBitbucketReview }>( + './bitbucket-read' + ); + jest.mocked(getBitbucketReview).mockImplementation(actual.getBitbucketReview); + expect(await run({ action: 'comment', body: 'Readable grant' })).toMatchObject({ + status: 'unresolved', + reason: 'no_atomic_revision_guard', + reference: { id: '2' }, + }); + expect(comments.map(value => value.content.raw)).toEqual(['Original', 'Readable grant']); + } +); + +it('AC1 keeps cached repositories readable through replacement, invalidation, and reconnect', async () => { + const timestamp = '2026-08-30 01:00:00.000+00'; + const credential = { + id: 'credential', + platform_integration_id: authorization.integrationId, + token_encrypted: 'ciphertext', + expires_at: null, + provider_credential_type: 'workspace_access_token', + provider_resource_id: null, + provider_base_url: null, + authorized_by_user_id: null, + provider_metadata: null, + provider_scopes: ['account', 'repository:write', 'pullrequest', 'webhook'], + provider_verified_at: timestamp, + credential_version: 1, + last_validated_at: timestamp, + last_used_at: null, + created_at: timestamp, + updated_at: timestamp, + }; + const row = { + integrationId: authorization.integrationId, + integrationStatus: 'active', + installationId: null, + workspaceUuid: repository.workspaceUuid, + workspaceSlug: 'team', + metadata: { displayName: 'Team' }, + repositories: [ + { + id: repository.repositoryId, + name: 'repo', + full_name: repository.fullName, + private: true, + default_branch: 'trunk', + }, + ], + repositoriesSyncedAt: timestamp, + authInvalidAt: null, + authInvalidReason: null, + credential, + }; + jest.mocked(db.select).mockImplementation( + () => + ({ + from: () => ({ leftJoin: () => ({ where: () => ({ limit: async () => [row] }) }) }), + }) as any + ); + const before = await getBitbucketWorkspaceAccessTokenStatus(authorization.owner.id); + expect(before).toMatchObject({ + status: 'connected', + recoveryAction: null, + reviewPermissions: { readReady: true, writeReady: false, recoveryAction: 'replace_token' }, + repositoryCache: { status: 'available', repositories: [{ fullName: 'team/repo' }] }, + }); + credential.provider_scopes.push('pullrequest:write'); + credential.credential_version++; + const after = await getBitbucketWorkspaceAccessTokenStatus(authorization.owner.id); + expect(after).toMatchObject({ + status: 'connected', + reviewPermissions: { readReady: true, writeReady: true, recoveryAction: null }, + }); + expect(after.repositoryCache).toEqual(before.repositoryCache); + + Object.assign(row, { authInvalidAt: timestamp, authInvalidReason: 'provider_rejected' }); + const invalidated = await getBitbucketWorkspaceAccessTokenStatus(authorization.owner.id); + expect(invalidated).toMatchObject({ + status: 'reconnect_required', + recoveryAction: 'replace_token', + reviewPermissions: null, + }); + expect(invalidated.repositoryCache).toEqual(before.repositoryCache); + + row.integrationId = '99999999-9999-4999-8999-999999999999'; + Object.assign(row, { authInvalidAt: null, authInvalidReason: null }); + credential.platform_integration_id = row.integrationId; + credential.provider_scopes = ['account', 'repository:write', 'pullrequest', 'webhook']; + credential.credential_version = 1; + expect(await getBitbucketWorkspaceAccessTokenStatus(authorization.owner.id)).toMatchObject({ + status: 'connected', + integrationId: row.integrationId, + reviewPermissions: { readReady: true, writeReady: false, recoveryAction: 'replace_token' }, + repositoryCache: { + status: 'available', + repositories: [{ fullName: 'team/repo', defaultBranch: 'trunk' }], + }, + }); +}); + +it.each( + (['source', 'destination'] as const).flatMap(endpoint => + (['branch', 'repository', 'workspace'] as const).flatMap(field => + (['immediate', 'task', 'lost response'] as const).map(mode => ({ endpoint, field, mode })) + ) + ) +)( + 'AC7 keeps same-SHA $endpoint $field drift unresolved through $mode', + async ({ endpoint, field, mode }) => { + const changeIdentity = () => { + if (field === 'branch') pr[endpoint].branch = { name: 'different' }; + else + pr[endpoint].repository = { + ...pr[endpoint].repository, + ...(field === 'repository' + ? { uuid: '{77777777-7777-4777-8777-777777777777}' } + : { workspace: { uuid: '{88888888-8888-4888-8888-888888888888}' } }), + }; + }; + pendingMerge = mode === 'task'; + lostResponse = mode === 'lost response'; + if (mode === 'immediate') afterWrite = changeIdentity; + const input: ReviewIntentInput = { action: 'merge', method: 'merge_commit' }; + let result = await run(input); + if (mode !== 'immediate') { + expect(result.status).toBe(mode === 'task' ? 'accepted' : 'unresolved'); + changeIdentity(); + lostResponse = false; + taskState = 'SUCCESS'; + result = await run(input, true); + } + expect(result).toMatchObject({ status: 'unresolved', retry: 'reconcile' }); + await run(input); + expect(events).toEqual(['merge']); + expect(merges).toHaveLength(1); + } +); + +it.each(['comment', 'approve', 'merge'] as const)( + 'AC6/AC7 resolves an abbreviated fork head for %s without a destination commit', + async action => { + const fork = { + ...repository, + repositoryId: '77777777-7777-4777-8777-777777777777', + workspaceUuid: '88888888-8888-4888-8888-888888888888', + fullName: 'contributor/fork', + }; + overview.source.repository = fork; + pr.source.repository = { + uuid: `{${fork.repositoryId}}`, + full_name: fork.fullName, + workspace: { uuid: `{${fork.workspaceUuid}}` }, + }; + pr.source.commit.hash = revision.headSha.slice(0, 12); + sourceInDestination = false; + const input: ReviewIntentInput = + action === 'comment' + ? { action, body: 'Fork review' } + : action === 'merge' + ? { action, method: 'merge_commit' } + : { action }; + expect(await run(input)).toMatchObject( + action === 'merge' + ? { status: 'confirmed', reference: { kind: 'review', id: '7' } } + : { status: 'unresolved', reason: 'no_atomic_revision_guard' } + ); + if (action === 'comment') expect(comments[1].content.raw).toBe('Fork review'); + if (action === 'approve') expect(state).toBe('approved'); + if (action === 'merge') expect(pr.state).toBe('MERGED'); + await run(input); + expect(events).toHaveLength(1); + } +); + +it.each([ + 'different full SHA', + 'ambiguous prefix', + 'missing fork evidence', + 'abbreviated evidence', +] as const)( + 'AC6 refuses %s instead of treating a source prefix as a full revision', + async condition => { + const other = `${revision.headSha.slice(0, 12)}${'c'.repeat(28)}`; + pr.source.commit.hash = revision.headSha.slice(0, 12); + sourceCommits = + condition === 'different full SHA' + ? [other] + : condition === 'ambiguous prefix' + ? [revision.headSha, other] + : condition === 'abbreviated evidence' + ? [pr.source.commit.hash] + : []; + if (condition === 'missing fork evidence') { + const fork = { + ...repository, + repositoryId: '77777777-7777-4777-8777-777777777777', + fullName: 'team/fork', + }; + overview.source.repository = fork; + pr.source.repository = { + ...nativeRepo, + uuid: `{${fork.repositoryId}}`, + full_name: fork.fullName, + }; + sourceInDestination = false; + } + expect(await run({ action: 'comment', body: 'Preserved draft' })).toMatchObject({ + status: 'rejected', + code: + condition === 'different full SHA' + ? 'conflict' + : condition === 'abbreviated evidence' + ? 'invalid_response' + : 'temporarily_unavailable', + retry: + condition === 'different full SHA' || condition === 'abbreviated evidence' + ? 'never' + : 'same-key', + }); + expect(events).toEqual([]); + expect(comments.map(value => value.content.raw)).toEqual(['Original']); + } +); + +it.each([false, true])( + 'AC6 resolves a later fork commit page or retains its retryable failure: %s', + async failPage => { + const fork = { + ...repository, + repositoryId: '77777777-7777-4777-8777-777777777777', + fullName: 'team/fork', + }; + overview.source.repository = fork; + pr.source.repository = { + ...nativeRepo, + uuid: `{${fork.repositoryId}}`, + full_name: fork.fullName, + }; + pr.source.commit.hash = revision.headSha.slice(0, 12); + sourceInDestination = false; + const next = `${taskUrl.split('/merge/')[0]}/commits?pagelen=50&page=2`; + const execute = auth.client.execute; + auth.client.execute = async input => { + if (input.operation !== 'commits') return execute(input); + if (input.next && failPage) + throw new BitbucketInteractiveClientError('temporarily_unavailable'); + return { + status: 200, + data: { values: [{ hash: input.next ? revision.headSha : revision.targetHeadSha }] }, + ...(input.next ? {} : { next }), + metadata: {}, + } as any; + }; + expect(await run({ action: 'comment', body: 'Paged fork review' })).toMatchObject( + failPage + ? { status: 'rejected', code: 'temporarily_unavailable', retry: 'same-key' } + : { status: 'unresolved', reason: 'no_atomic_revision_guard', reference: { id: '2' } } + ); + expect(comments.map(value => value.content.raw)).toEqual( + failPage ? ['Original'] : ['Original', 'Paged fork review'] + ); + } +); + +it('AC6 preserves closed same-repository comments when the source branch and commit list are absent', async () => { + pr.state = 'MERGED'; + pr.source.branch = null; + pr.source.commit.hash = revision.headSha.slice(0, 12); + overview.state = 'merged'; + overview.source.branch = null; + sourceCommits = []; + expect(await run({ action: 'comment', body: 'Closed review' })).toMatchObject({ + status: 'unresolved', + reason: 'no_atomic_revision_guard', + reference: { id: '2' }, + }); + expect(comments.map(value => value.content.raw)).toEqual(['Original', 'Closed review']); +}); + +const canonicalTaskUrl = + 'https://api.bitbucket.org/2.0/repositories/team/repo/pullrequests/7/merge/task-status/task-1'; + +it('AC7 retains canonical 202 task locations through the production SDK', async () => { + pendingMerge = true; + taskLocation = canonicalTaskUrl; + taskSelf = canonicalTaskUrl; + const input: ReviewIntentInput = { action: 'merge', method: 'merge_commit' }; + const result = await run(input); + expect(events).toEqual(['merge']); + expect(result).toMatchObject({ + status: 'accepted', + reference: { id: 'task-1', url: canonicalTaskUrl }, + task: { state: 'pending' }, + }); + taskState = 'SUCCESS'; + expect(await run(input, true)).toMatchObject({ status: 'confirmed' }); + expect(merges).toHaveLength(1); +}); + +it('AC7 polls documented canonical task self links after a UUID-addressed merge', async () => { + pendingMerge = true; + taskSelf = canonicalTaskUrl; + const input: ReviewIntentInput = { action: 'merge', method: 'merge_commit' }; + expect(await run(input)).toMatchObject({ status: 'accepted', reference: { url: taskUrl } }); + expect(await run(input, true)).toMatchObject({ status: 'accepted', task: { state: 'pending' } }); + taskState = 'SUCCESS'; + expect(await run(input, true)).toMatchObject({ status: 'confirmed' }); + expect(pr.state).toBe('MERGED'); + expect(events).toEqual(['merge']); +}); + +it.each([ + ['origin', canonicalTaskUrl.replace('api.bitbucket.org', 'example.com')], + ['scheme', canonicalTaskUrl.replace('https:', 'http:')], + ['port', canonicalTaskUrl.replace('.org/', '.org:8443/')], + ['credentials', canonicalTaskUrl.replace('https://', 'https://user@')], + ['workspace', canonicalTaskUrl.replace('/team/', '/other/')], + ['repository', canonicalTaskUrl.replace('/repo/', '/other/')], + ['review', canonicalTaskUrl.replace('/7/', '/8/')], + ['task', canonicalTaskUrl.replace('task-1', 'task-2')], + ['query', `${canonicalTaskUrl}?other=1`], + ['fragment', `${canonicalTaskUrl}#other`], + ['encoded slash', `${canonicalTaskUrl}%2Fother`], + ['extra path', `${canonicalTaskUrl}/other`], +])('AC7 rejects a canonical task link with a different %s', async (_field, self) => { + pendingMerge = true; + const input: ReviewIntentInput = { action: 'merge', method: 'merge_commit' }; + await run(input); + taskSelf = self; + taskState = 'SUCCESS'; + expect(await run(input, true)).toMatchObject({ status: 'unresolved', retry: 'reconcile' }); + expect([...records.values()][0].result).toMatchObject({ + status: 'accepted', + reference: { id: 'task-1' }, + }); + await run(input); + expect(events).toEqual(['merge']); +}); diff --git a/apps/web/src/lib/provider-review/bitbucket-write.ts b/apps/web/src/lib/provider-review/bitbucket-write.ts new file mode 100644 index 0000000000..7fe6d07351 --- /dev/null +++ b/apps/web/src/lib/provider-review/bitbucket-write.ts @@ -0,0 +1,831 @@ +import 'server-only'; + +import { z } from 'zod'; +import { repositoryResourceKey } from '@kilocode/app-shared/code-review/repository-identity'; +import { + BitbucketMergeEvidenceSchema, + ReviewIntentInputSchema, + ReviewRevisionSchema, + serializeReviewWriteRequest, + type BitbucketMergeEvidence, + type ProviderReference, + type ReviewIntentInput, + type ReviewMutationResult, + type ReviewOverview, + type ReviewPosition, + type ReviewRevision, +} from '@kilocode/app-shared/provider-review'; +import { + BitbucketInteractiveClientError, + type BitbucketInteractiveRequest, +} from '@/lib/integrations/platforms/bitbucket/interactive-client'; +import { + BitbucketPathSchema, + BitbucketUuidSchema, + assertBitbucketReviewIdentity, + parseBitbucket, + type BitbucketReviewAuthorization, +} from './bitbucket-authorization'; +import { getBitbucketReview, listBitbucketFiles } from './bitbucket-read'; +import { + confirmedReviewEffect, + rejectedReviewEffect, + unresolvedReviewEffect, + runReviewOperation, + type ReviewEffectResult, + type ReviewOperationRequest, +} from './operation'; + +const id = z.number().int().positive().max(Number.MAX_SAFE_INTEGER); +const sha = z.string().regex(/^[a-f0-9]{40}$/); +const providerSha = z.string().regex(/^[a-f0-9]{7,40}$/); +const revisionSchema = ReviewRevisionSchema.extend({ + headSha: sha, + targetHeadSha: sha, + baseSha: sha.nullable(), + startSha: z.null(), +}); +const endpoint = z.object({ + repository: z + .object({ + uuid: BitbucketUuidSchema, + full_name: z.string(), + workspace: z.object({ uuid: BitbucketUuidSchema }).optional(), + }) + .nullable(), + branch: z.object({ name: BitbucketPathSchema }).nullable(), + commit: z.object({ hash: providerSha }), +}); +const reviewSchema = z.object({ + type: z.literal('pullrequest'), + id, + state: z.enum(['OPEN', 'MERGED', 'DECLINED', 'SUPERSEDED']), + links: z.object({ html: z.object({ href: z.string() }) }), + source: endpoint, + destination: endpoint, + merge_commit: z.object({ hash: providerSha }).nullish(), +}); +const inlineSchema = z.object({ + path: BitbucketPathSchema, + from: id.nullish(), + to: id.nullish(), + start_from: id.nullish(), + start_to: id.nullish(), +}); +const commentSchema = z.object({ + type: z.literal('pullrequest_comment'), + id, + content: z.object({ raw: z.string() }).nullish(), + parent: z.object({ id }).nullish(), + inline: inlineSchema.nullish(), + pullrequest: z.object({ id }).optional(), + user: z.object({ uuid: BitbucketUuidSchema }).nullish(), + deleted: z.boolean().optional(), + resolution: z.object({}).nullish(), +}); +const methodSchema = z.enum([ + 'merge_commit', + 'squash', + 'fast_forward', + 'squash_fast_forward', + 'rebase_fast_forward', + 'rebase_merge', +]); +const fields: Partial> = { + comment: ['body'], + inlineComment: ['body', 'position'], + reply: ['body', 'target'], + resolveThread: ['target'], + reopenThread: ['target'], + approve: [], + unapprove: [], + requestChanges: [], + removeChangeRequest: [], + merge: ['method', 'commitTitle', 'commitMessage', 'deletion'], + submitReview: ['body', 'comments', 'choice'], +}; +type Context = { auth: BitbucketReviewAuthorization; request: ReviewOperationRequest }; +type Effect = { itemId: string; input: ReviewIntentInput }; +function conflict(): never { + throw new BitbucketInteractiveClientError('conflict'); +} +function heads(expected: ReviewRevision, actual: ReviewRevision) { + if (expected.headSha !== actual.headSha || expected.targetHeadSha !== actual.targetHeadSha) + conflict(); +} +function path({ auth, request }: Context) { + return { + ...auth.path, + pull_request_id: parseBitbucket(id, Number(request.intent.review.number), 'invalid_request'), + }; +} +function reference( + context: Context, + kind: ProviderReference['kind'], + value = context.request.intent.review.reviewId +): ProviderReference { + const url = context.request.intent.review.canonicalUrl; + return { + provider: 'bitbucket', + kind, + id: value, + url: kind === 'comment' || kind === 'thread' ? `${url}/_/diff#comment-${value}` : url, + }; +} +function target(context: Context, input: ReviewIntentInput) { + const value = input.target; + if ( + !value || + value.provider !== 'bitbucket' || + !['comment', 'thread'].includes(value.kind) || + !/^[1-9]\d*$/.test(value.id) || + (value.url !== null && value.url !== reference(context, value.kind, value.id).url) + ) + throw new BitbucketInteractiveClientError('invalid_request'); + return parseBitbucket(id, Number(value.id), 'invalid_request'); +} +function inline(position: ReviewPosition, revision: ReviewRevision) { + const native = position.native; + if (native.provider !== 'bitbucket') throw new BitbucketInteractiveClientError('invalid_request'); + parseBitbucket(revisionSchema, position.revision, 'invalid_request'); + heads(revision, position.revision); + if ( + (revision.baseSha !== null && position.revision.baseSha !== revision.baseSha) || + position.revision.startSha !== null || + position.line !== (position.side === 'old' ? native.from : native.to) || + position.startLine !== (position.startSide === 'old' ? native.startFrom : native.startTo) || + (position.startLine === undefined && + (native.startFrom !== undefined || native.startTo !== undefined)) || + (position.startSide === position.side && + position.startLine !== undefined && + position.startLine > position.line) + ) + conflict(); + return { + path: parseBitbucket( + BitbucketPathSchema, + position.side === 'old' ? position.oldPath : position.newPath, + 'invalid_request' + ), + ...(native.from === undefined ? {} : { from: native.from }), + ...(native.to === undefined ? {} : { to: native.to }), + ...(native.startFrom === undefined ? {} : { start_from: native.startFrom }), + ...(native.startTo === undefined ? {} : { start_to: native.startTo }), + }; +} +function review(context: Context, value: unknown) { + const result = parseBitbucket(reviewSchema, value); + const expected = context.auth.repository; + if ( + result.id !== path(context).pull_request_id || + result.links.html.href !== context.request.intent.review.canonicalUrl || + result.destination.repository?.uuid !== expected.repositoryId || + result.destination.repository.full_name !== expected.fullName || + (result.destination.repository.workspace && + result.destination.repository.workspace.uuid !== expected.workspaceUuid) + ) + throw new BitbucketInteractiveClientError('repository_mismatch'); + return result; +} +async function snapshot(context: Context) { + const response = await context.auth.client.execute({ + operation: 'pullRequest', + params: { + path: path(context), + query: { fields: '+source.repository.workspace,+destination.repository.workspace' }, + }, + }); + if (response.status !== 200) throw new BitbucketInteractiveClientError('invalid_response'); + return review(context, response.data); +} +async function fullSha(context: Context, hash: string) { + if (hash.length === 40) return hash; + const response = await context.auth.client.execute({ + operation: 'commit', + params: { path: { ...context.auth.path, commit: hash } }, + }); + if (response.status !== 200) throw new BitbucketInteractiveClientError('invalid_response'); + const commit = parseBitbucket(z.object({ hash: sha }), response.data); + if (!commit.hash.startsWith(hash)) conflict(); + return commit.hash; +} +async function fullSourceSha(context: Context, source: z.infer) { + const hash = source.commit.hash; + if (hash.length === 40) return hash; + let next: string | undefined; + let candidate: string | undefined; + const seen = new Set(); + do { + // Destination authorization permits the PR's commits, not an arbitrary source repository lookup. + const response = await context.auth.client.execute({ + operation: 'commits', + params: { path: path(context) }, + ...(next ? { next } : {}), + }); + if (response.status !== 200) throw new BitbucketInteractiveClientError('invalid_response'); + const page = parseBitbucket( + z.object({ values: z.array(z.object({ hash: sha })).max(50) }), + response.data + ); + for (const commit of page.values) { + if (!commit.hash.startsWith(hash)) continue; + if (candidate && candidate !== commit.hash) + throw new BitbucketInteractiveClientError('temporarily_unavailable'); + candidate = commit.hash; + } + next = response.next; + if (next) { + if (seen.has(next)) throw new BitbucketInteractiveClientError('invalid_pagination'); + if (seen.size >= 99) throw new BitbucketInteractiveClientError('page_limit_exceeded'); + seen.add(next); + } + } while (next); + if (candidate) return candidate; + // Closed same-repository PRs can have an empty commits list after source deletion. + if (source.repository?.uuid === context.auth.repository.repositoryId) + return fullSha(context, hash); + throw new BitbucketInteractiveClientError('temporarily_unavailable'); +} +async function checkHeads(context: Context, value: z.infer) { + heads(context.request.intent.revision, { + headSha: await fullSourceSha(context, value.source), + targetHeadSha: await fullSha(context, value.destination.commit.hash), + baseSha: null, + startSha: null, + }); +} +async function comment(context: Context, commentId: number) { + const response = await context.auth.client.execute({ + operation: 'comment', + params: { path: { ...path(context), comment_id: commentId } }, + }); + if (response.status !== 200) throw new BitbucketInteractiveClientError('invalid_response'); + const value = parseBitbucket(commentSchema, response.data); + if ( + value.id !== commentId || + value.deleted || + (value.pullrequest && value.pullrequest.id !== path(context).pull_request_id) + ) + conflict(); + return value; +} +function checkCapability(overview: ReviewOverview, action: ReviewIntentInput['action']) { + const value = overview.authorization.capabilities[action]; + if (value.permission === 'forbidden') + throw new BitbucketInteractiveClientError('insufficient_permissions'); + if ( + value.support !== 'supported' || + value.version !== 'available' || + value.license !== 'available' || + value.restrictions.length + ) + conflict(); + // Unknown OAuth merge permission is not permission denial. The provider enforces it on the write. +} +async function preflight(context: Context, input: ReviewIntentInput) { + const { + auth, + request: { intent }, + } = context; + const overview = await getBitbucketReview(auth, intent.review.number); + assertBitbucketReviewIdentity(auth, overview.identity); + heads(intent.revision, overview.revision); + checkCapability(overview, input.action); + if (input.position) { + let cursor; + let found = false; + do { + const page = await listBitbucketFiles(auth, intent.review, intent.revision, cursor); + found = page.items.some( + file => + file.oldPath === input.position?.oldPath && + file.newPath === input.position.newPath && + file.revision.baseSha === input.position.revision.baseSha + ); + cursor = page.nextCursor; + } while (!found && cursor); + if (!found) conflict(); + } + if (input.target) await comment(context, target(context, input)); + if (input.action === 'merge') { + if ( + overview.state !== 'open' || + !overview.merge.methods.some(method => method.id === input.method) + ) + conflict(); + if (input.deletion?.effect === 'delete') { + checkCapability(overview, 'deleteBranch'); + const source = overview.source; + if ( + !source.repository || + source.repository.repositoryId !== auth.repository.repositoryId || + source.repository.provider !== 'bitbucket' || + source.repository.workspaceUuid !== auth.repository.workspaceUuid || + !source.branch || + source.branch === overview.target.branch || + !auth.repository.defaultBranch || + source.branch === auth.repository.defaultBranch || + input.deletion.branch !== source.branch || + input.deletion.expectedHeadSha !== intent.revision.headSha || + input.deletion.repositoryKey !== + repositoryResourceKey(auth.userId, { + repository: source.repository, + authorization: auth.authorization, + }) + ) + conflict(); + const response = await auth.client.execute({ + operation: 'branch', + params: { path: { ...auth.path, name: source.branch } }, + }); + if (response.status !== 200) conflict(); + const branch = parseBitbucket( + z.object({ name: z.string(), target: z.object({ hash: providerSha }) }), + response.data + ); + if ( + branch.name !== source.branch || + (await fullSha(context, branch.target.hash)) !== intent.revision.headSha + ) + conflict(); + } + } + const before = await snapshot(context); + await checkHeads(context, before); + if ( + ['approve', 'unapprove', 'requestChanges', 'removeChangeRequest', 'merge'].includes( + input.action + ) && + before.state !== 'OPEN' + ) + conflict(); + if ( + before.source.repository?.uuid !== overview.source.repository?.repositoryId || + (before.source.branch?.name ?? null) !== overview.source.branch || + before.destination.branch?.name !== overview.target.branch + ) + conflict(); + return before; +} +function taskReference(context: Context, location: string): ProviderReference { + const url = new URL(location); + // The API returns canonical slug links even when the authorized request uses UUIDs. + const repositories = [ + `${encodeURIComponent(context.auth.path.workspace)}/${encodeURIComponent(context.auth.path.repo_slug)}`, + context.auth.repository.fullName.split('/').map(encodeURIComponent).join('/'), + ]; + const prefix = repositories + .map( + repository => + `/2.0/repositories/${repository}/pullrequests/${path(context).pull_request_id}/merge/task-status/` + ) + .find(value => url.pathname.startsWith(value)); + if (!prefix) throw new BitbucketInteractiveClientError('invalid_response'); + const taskId = decodeURIComponent(url.pathname.slice(prefix.length)); + if ( + url.origin !== 'https://api.bitbucket.org' || + url.username || + url.password || + url.search || + url.hash || + !/^[A-Za-z0-9_{}.-]+$/.test(taskId) || + taskId === '.' || + taskId === '..' + ) + throw new BitbucketInteractiveClientError('invalid_response'); + return { provider: 'bitbucket', kind: 'merge-task', id: taskId, url: url.href }; +} +function pending(ref: ProviderReference): ReviewEffectResult { + return { + status: 'accepted', + reference: ref, + retry: 'reconcile', + reconciliation: 'pending', + task: { + reference: { ...ref, provider: 'bitbucket', kind: 'merge-task' }, + state: 'pending', + mergeCommitSha: null, + error: null, + }, + }; +} +function mergeIdentity(value: z.infer): BitbucketMergeEvidence { + const identity = (value: z.infer) => ({ + repositoryId: value.repository?.uuid, + workspaceUuid: value.repository?.workspace?.uuid, + fullName: value.repository?.full_name, + branch: value.branch?.name, + }); + return parseBitbucket(BitbucketMergeEvidenceSchema, { + source: identity(value.source), + destination: identity(value.destination), + }); +} +async function merged( + context: Context, + evidence: BitbucketMergeEvidence | null, + returned?: unknown +): Promise { + const ref = reference(context, 'review'); + if (!evidence) return unresolvedReviewEffect('merge_identity_unavailable', ref); + const current = await snapshot(context); + const result = returned === undefined ? current : review(context, returned); + if ( + result.state !== 'MERGED' || + current.state !== 'MERGED' || + !result.merge_commit || + !current.merge_commit + ) + return unresolvedReviewEffect('merge_not_confirmed', ref); + for (const value of [result, current]) { + for (const side of ['source', 'destination'] as const) { + const endpoint = value[side]; + const expected = evidence[side]; + if ( + endpoint.repository?.uuid !== expected.repositoryId || + endpoint.repository.workspace?.uuid !== expected.workspaceUuid || + endpoint.repository.full_name !== expected.fullName || + endpoint.branch?.name !== expected.branch + ) + conflict(); + } + } + await checkHeads(context, result); + await checkHeads(context, current); + if ( + (await fullSha(context, result.merge_commit.hash)) !== + (await fullSha(context, current.merge_commit.hash)) + ) + conflict(); + return confirmedReviewEffect(ref); +} +async function reconcile( + context: Context, + input: ReviewIntentInput, + stored: ReviewEffectResult | null, + evidence?: BitbucketMergeEvidence | null +): Promise { + const ref = stored && 'reference' in stored ? stored.reference : null; + try { + if (input.action === 'merge') { + if (!evidence) return unresolvedReviewEffect('merge_identity_unavailable', ref); + if (ref?.kind !== 'merge-task') return await merged(context, evidence); + if (ref.provider !== 'bitbucket' || !ref.url || taskReference(context, ref.url).id !== ref.id) + conflict(); + const response = await context.auth.client.execute({ + operation: 'mergeTask', + params: { path: { ...path(context), task_id: ref.id } }, + }); + if (response.status !== 200) throw new BitbucketInteractiveClientError('invalid_response'); + const task = parseBitbucket( + z.object({ + task_status: z.enum(['PENDING', 'SUCCESS']), + links: z.object({ self: z.object({ href: z.string() }) }), + merge_result: z.unknown().optional(), + }), + response.data + ); + if (taskReference(context, task.links.self.href).id !== ref.id) conflict(); + if (task.task_status === 'PENDING') return pending(ref); + if (!task.merge_result) return unresolvedReviewEffect('merge_result_missing', ref); + return await merged(context, evidence, task.merge_result); + } + if (!ref) return unresolvedReviewEffect('provider_receipt_missing'); + if (ref.kind === 'comment' || ref.kind === 'thread') await comment(context, Number(ref.id)); + await checkHeads(context, await snapshot(context)); + // Bitbucket PR comments and participant states have no immutable reviewed SHA. Matching + // pre/post heads cannot upgrade a reference chosen before dispatch into a provider receipt. + return stored?.status === 'unresolved' && stored.reason === 'no_atomic_revision_guard' + ? stored + : unresolvedReviewEffect('provider_outcome_unknown', ref); + } catch { + return unresolvedReviewEffect('provider_outcome_unknown', ref); + } +} +async function perform( + context: Context, + input: ReviewIntentInput, + persistMergeEvidence?: (evidence: BitbucketMergeEvidence) => Promise +): Promise { + let dispatched = false, + responded = false; + let ref: ProviderReference | null = null; + try { + const before = await preflight(context, input); + const evidence = input.action === 'merge' ? mergeIdentity(before) : null; + const params = { path: path(context) }; + let operation: BitbucketInteractiveRequest; + switch (input.action) { + case 'comment': + case 'inlineComment': + case 'reply': + operation = { + operation: 'createComment', + params, + body: { + type: 'pullrequest_comment', + content: { raw: input.body }, + ...(input.position + ? { inline: inline(input.position, context.request.intent.revision) } + : {}), + ...(input.action === 'reply' + ? { parent: { type: 'comment', id: target(context, input) } } + : {}), + }, + }; + break; + case 'resolveThread': + case 'reopenThread': + ref = reference(context, 'thread', String(target(context, input))); + operation = { + operation: input.action === 'resolveThread' ? 'resolveComment' : 'reopenComment', + params: { path: { ...params.path, comment_id: target(context, input) } }, + }; + break; + case 'approve': + case 'unapprove': + case 'requestChanges': + case 'removeChangeRequest': + ref = reference(context, 'review'); + operation = { operation: input.action, params }; + break; + case 'merge': { + const message = [input.commitTitle, input.commitMessage] + .filter(value => value !== undefined) + .join('\n\n'); + if (Buffer.byteLength(message, 'utf8') > 128 * 1024) + throw new BitbucketInteractiveClientError('request_too_large'); + operation = { + operation: 'merge', + params, + body: { + type: 'pullrequest_merge_parameters', + merge_strategy: methodSchema.parse(input.method), + close_source_branch: input.deletion?.effect === 'delete', + ...(message ? { message } : {}), + }, + }; + break; + } + default: + throw new BitbucketInteractiveClientError('invalid_request'); + } + if (evidence) { + if (!persistMergeEvidence) throw new Error('Merge evidence persistence is required'); + await persistMergeEvidence(evidence); + } + // No If-Match or expected-head header exists for these Bitbucket operations. + dispatched = true; + const response = await context.auth.client.execute(operation); + responded = true; + if (input.action === 'merge') { + if (response.status === 202) return pending(taskReference(context, response.location)); + if (response.status !== 200) throw new BitbucketInteractiveClientError('invalid_response'); + ref = reference(context, 'review'); + return await merged(context, evidence, response.data); + } + if (operation.operation === 'createComment') { + if (response.status !== 201) throw new BitbucketInteractiveClientError('invalid_response'); + const value = parseBitbucket(commentSchema, response.data); + if ( + value.pullrequest?.id !== params.path.pull_request_id || + value.content?.raw !== input.body || + value.deleted || + (value.parent?.id ?? null) !== (input.action === 'reply' ? target(context, input) : null) + ) + conflict(); + const selected = input.position + ? inline(input.position, context.request.intent.revision) + : null; + if ( + selected + ? !value.inline || + (['path', 'from', 'to', 'start_from', 'start_to'] as const).some( + key => (value.inline?.[key] ?? undefined) !== selected[key] + ) + : value.inline != null + ) + conflict(); + if ( + context.auth.credentialKind === 'bitbucketOAuth' && + value.user?.uuid !== context.auth.actor.id + ) + conflict(); + ref = reference(context, 'comment', String(value.id)); + } else if (input.action === 'approve' || input.action === 'requestChanges') { + const participant = parseBitbucket( + z.object({ + user: z.object({ uuid: BitbucketUuidSchema }), + state: z.enum(['approved', 'changes_requested']).nullish(), + approved: z.boolean().optional(), + }), + response.data + ); + if ( + response.status !== 200 || + (input.action === 'approve' + ? !(participant.state === 'approved' || participant.approved) + : participant.state !== 'changes_requested') || + (context.auth.credentialKind === 'bitbucketOAuth' && + participant.user.uuid !== context.auth.actor.id) + ) + conflict(); + } else if (input.action === 'resolveThread' || input.action === 'reopenThread') { + const value = await comment(context, target(context, input)); + if ((value.resolution != null) !== (input.action === 'resolveThread')) conflict(); + } else if (response.status !== 204) + throw new BitbucketInteractiveClientError('invalid_response'); + const after = await snapshot(context); + await checkHeads(context, after); + if ( + before.state !== after.state || + before.source.repository?.uuid !== after.source.repository?.uuid || + before.source.branch?.name !== after.source.branch?.name || + before.destination.branch?.name !== after.destination.branch?.name + ) + conflict(); + return unresolvedReviewEffect('no_atomic_revision_guard', ref); + } catch (error) { + if ( + error instanceof BitbucketInteractiveClientError && + (!dispatched || + (!responded && + [ + 'insufficient_permissions', + 'authentication_rejected', + 'not_connected', + // reconnect_required can also come from the authorization fence after a write. + 'conflict', + 'not_found', + 'request_too_large', + ].includes(error.code))) + ) + return rejectedReviewEffect( + error.code, + [ + 'temporarily_unavailable', + 'provider_unavailable', + 'rate_limited', + 'request_timed_out', + 'transport_failed', + ].includes(error.code) + ? 'same-key' + : 'never' + ); + return dispatched + ? unresolvedReviewEffect('provider_outcome_unknown', ref) + : rejectedReviewEffect('preflight_unavailable', 'same-key'); + } +} +async function deletion(context: Context, merge: ReviewEffectResult): Promise { + const selected = context.request.intent.input.deletion; + if (merge.status !== 'confirmed' || !selected) + return unresolvedReviewEffect('merge_not_confirmed'); + // close_source_branch requests deletion but returns no deletion receipt. Observe separately; + // never issue a second delete against a branch that can have advanced or been recreated. + try { + await context.auth.client.execute({ + operation: 'branch', + params: { path: { ...context.auth.path, name: selected.branch } }, + }); + return unresolvedReviewEffect('source_branch_still_present'); + } catch (error) { + if (error instanceof BitbucketInteractiveClientError && error.code === 'not_found') { + try { + // A masked repository-access failure is not evidence that the branch was deleted. + const current = await snapshot(context); + if (current.state === 'MERGED') return confirmedReviewEffect(reference(context, 'review')); + } catch { + /* Keep the confirmed merge separate from unavailable deletion evidence. */ + } + } + return unresolvedReviewEffect('source_branch_deletion_unknown'); + } +} + +export type BitbucketReviewOperationRequest = ReviewOperationRequest & { + intent: { revision: z.infer }; +}; + +/** + * Use fresh authorizeBitbucketReview authorization for submission and status checks. + * Bitbucket has no atomic expected-head guard. Comments and participant receipts remain + * revision-unresolved even when both heads match; merge confirmation requires provider readback. + */ +export async function runBitbucketReviewOperation( + auth: BitbucketReviewAuthorization, + request: BitbucketReviewOperationRequest, + statusOnly = false +): Promise { + const { intent } = request; + if ( + request.effect || + request.userId !== auth.userId || + intent.accountId !== auth.userId || + intent.actorId !== auth.actor.id + ) + return rejectedReviewEffect('operation_identity_mismatch'); + assertBitbucketReviewIdentity(auth, intent.review); + serializeReviewWriteRequest(intent); + parseBitbucket(revisionSchema, intent.revision, 'invalid_request'); + const input = parseBitbucket(ReviewIntentInputSchema, intent.input, 'invalid_request'); + const allowed = fields[input.action]; + if (!allowed || Object.keys(input).some(key => key !== 'action' && !allowed.includes(key))) + throw new BitbucketInteractiveClientError('invalid_request'); + const comments = input.comments ?? []; + if ( + comments.length > 100 || + new Set(comments.map(value => value.itemId)).size !== comments.length + ) + throw new BitbucketInteractiveClientError('invalid_request'); + const effects: Effect[] = + input.action === 'submitReview' + ? [ + ...comments.map(value => ({ + itemId: `comment:${value.itemId}`, + input: { action: 'inlineComment' as const, body: value.body, position: value.position }, + })), + ...(input.body + ? [{ itemId: 'summary', input: { action: 'comment' as const, body: input.body } }] + : []), + ...(input.choice && input.choice !== 'comment' + ? [{ itemId: 'decision', input: { action: input.choice } }] + : []), + ] + : [{ itemId: input.action, input }]; + const context = { auth, request }; + for (const effect of effects) { + parseBitbucket(z.string().min(1).max(512), effect.itemId, 'invalid_request'); + if ( + ['comment', 'inlineComment', 'reply'].includes(effect.input.action) && + !effect.input.body?.trim() + ) + throw new BitbucketInteractiveClientError('invalid_request'); + if (effect.input.action === 'inlineComment' && !effect.input.position) + throw new BitbucketInteractiveClientError('invalid_request'); + if (effect.input.position) inline(effect.input.position, intent.revision); + if (['reply', 'resolveThread', 'reopenThread'].includes(effect.input.action)) + target(context, effect.input); + } + if (input.action === 'merge') parseBitbucket(methodSchema, input.method, 'invalid_request'); + const execute = (effect: Effect, child: boolean, reconcileOnly = statusOnly) => + runReviewOperation( + child ? { ...request, effect: { id: effect.itemId, action: effect.input.action } } : request, + { + ...(reconcileOnly + ? {} + : { + execute: ( + persistMergeEvidence?: (evidence: BitbucketMergeEvidence) => Promise + ) => perform(context, effect.input, persistMergeEvidence), + }), + reconcile: (stored, evidence) => reconcile(context, effect.input, stored, evidence), + } + ); + if (input.action !== 'submitReview' && input.deletion?.effect !== 'delete') + return execute(effects[0], false); + const items: Extract['items'] = []; + const publish = async (reconcileOnly = statusOnly): Promise => { + let stopped = false; + for (const effect of effects) { + const result: ReviewEffectResult = stopped + ? rejectedReviewEffect('previous_effect_unconfirmed', 'same-key') + : await execute(effect, true, reconcileOnly); + items.push({ itemId: effect.itemId, effect: effect.input.action, result }); + // A verified receipt permits the other independent effects, but never claims atomic review success. + stopped ||= + result.status !== 'confirmed' && + !(result.status === 'unresolved' && result.reason === 'no_atomic_revision_guard'); + if (effect.input.action === 'merge' && input.deletion?.effect === 'delete') { + const observe = () => deletion(context, result); + items.push({ + itemId: 'deleteBranch', + effect: 'deleteBranch', + result: await runReviewOperation( + { ...request, effect: { id: 'deleteBranch', action: 'deleteBranch' } }, + { + ...(reconcileOnly ? {} : { execute: observe }), + reconcile: observe, + } + ), + }); + } + } + return items.some(item => item.result.status !== 'confirmed') + ? unresolvedReviewEffect('batch_incomplete') + : confirmedReviewEffect(effects.length ? reference(context, 'review') : null); + }; + const result = await runReviewOperation(request, { + ...(statusOnly ? {} : { execute: () => publish() }), + reconcile: () => publish(), + aggregate: true, + }); + // Old aggregate confirmations cannot replace the merge effect's preflight evidence. + // Retain this fallback until old clients/records disappear and the 30-day ledger window expires. + // Status-only child checks also prevent a missing legacy child row from admitting another write. + if (input.action === 'merge' && result.status === 'confirmed' && items.length === 0) + await publish(true); + return items.some(item => item.result.status !== 'confirmed') + ? { status: 'partial', items, retry: 'unfinished-only', reconciliation: 'required' } + : result; +} diff --git a/apps/web/src/lib/provider-review/operation.test.ts b/apps/web/src/lib/provider-review/operation.test.ts index 39342b54a2..d1e724bda5 100644 --- a/apps/web/src/lib/provider-review/operation.test.ts +++ b/apps/web/src/lib/provider-review/operation.test.ts @@ -1,4 +1,9 @@ jest.mock('@/lib/drizzle', () => ({ db: { select: jest.fn() } })); +jest.mock('@/lib/config.server', () => ({})); +jest.mock('./bitbucket-read', () => ({ + getBitbucketReview: jest.fn(), + listBitbucketFiles: jest.fn(), +})); jest.mock('@kilocode/db/operation-ledger', () => ({ ...jest.requireActual('@kilocode/db/operation-ledger'), admitOperation: jest.fn(), @@ -9,8 +14,13 @@ jest.mock('@kilocode/db/operation-ledger', () => ({ })); import { PgDialect } from 'drizzle-orm/pg-core'; +import { repositoryResourceKey } from '@kilocode/app-shared/code-review/repository-identity'; import { db } from '@/lib/drizzle'; -import { user_terms_acceptances, type OperationLedgerRow } from '@kilocode/db/schema'; +import { + operation_ledgers, + user_terms_acceptances, + type OperationLedgerRow, +} from '@kilocode/db/schema'; import { admitOperation, recordOperationProgress, @@ -21,7 +31,17 @@ import { } from '@kilocode/db/operation-ledger'; import { ANALYTICS_EVENT_SCHEMAS } from '@kilocode/app-shared/analytics'; import { CURRENT_UGC_TERMS_VERSION } from '@kilocode/app-shared/moderation'; -import { providerReviewFixtures } from '@kilocode/app-shared/provider-review/fixtures'; +import { + providerReviewFixtures, + reviewCapabilityFixtures, +} from '@kilocode/app-shared/provider-review/fixtures'; +import { BitbucketInteractiveClientError } from '@/lib/integrations/platforms/bitbucket/interactive-client'; +import type { BitbucketReviewAuthorization } from './bitbucket-authorization'; +import { getBitbucketReview } from './bitbucket-read'; +import { + runBitbucketReviewOperation, + type BitbucketReviewOperationRequest, +} from './bitbucket-write'; import { confirmedReviewEffect, rejectedReviewEffect, @@ -524,3 +544,442 @@ it('AC6 blocks dispatch when the durable dispatch fence cannot persist', async ( expect(await run()).toMatchObject({ status: 'unresolved', retry: 'reconcile' }); expect(effects).toEqual([]); }); + +describe('Bitbucket merge evidence through the real operation and JSON ledger boundary', () => { + const repository = { + provider: 'bitbucket' as const, + instanceUrl: 'https://bitbucket.org', + repositoryId: '44444444-4444-4444-8444-444444444444', + workspaceUuid: '33333333-3333-4333-8333-333333333333', + fullName: 'team/repo', + defaultBranch: 'trunk', + }; + const authorization = { + kind: 'ownerIntegration' as const, + owner: { type: 'org' as const, id: '11111111-1111-4111-8111-111111111111' }, + integrationId: '22222222-2222-4222-8222-222222222222', + }; + const mergeRequest: BitbucketReviewOperationRequest = { + userId, + distinctId: 'caller', + operationKey: '66666666-6666-4666-8666-666666666666', + intent: { + accountId: userId, + actorId: '55555555-5555-4555-8555-555555555555', + review: { + repository, + authorization, + reviewId: '7', + number: '7', + canonicalUrl: 'https://bitbucket.org/team/repo/pull-requests/7', + }, + revision: { + headSha: 'a'.repeat(40), + targetHeadSha: 'b'.repeat(40), + baseSha: null, + startSha: null, + }, + input: { action: 'merge', method: 'merge_commit' }, + }, + }; + const evidence = { + source: { + repositoryId: repository.repositoryId, + workspaceUuid: repository.workspaceUuid, + fullName: repository.fullName, + branch: 'feature', + }, + destination: { + repositoryId: repository.repositoryId, + workspaceUuid: repository.workspaceUuid, + fullName: repository.fullName, + branch: 'trunk', + }, + }; + const taskUrl = + 'https://api.bitbucket.org/2.0/repositories/team/repo/pullrequests/7/merge/task-status/task-1'; + const actualLedger = jest.requireActual<{ + recordOperationProgress: typeof recordOperationProgress; + recordOperationAcceptance: typeof recordOperationAcceptance; + }>('@kilocode/db/operation-ledger'); + let auth: BitbucketReviewAuthorization; + let pr: any; + let acceptedTask: boolean; + let taskComplete: boolean; + let unavailable: boolean; + let loseResponse: boolean; + let dispatchSnapshots: unknown[]; + let recoveredRun: typeof runBitbucketReviewOperation; + + // Keep real progress/acceptance merging and the production JSONB serializer. Only storage I/O is fake. + function storage(rowId: string) { + return { + select: () => ({ + from: () => ({ where: () => ({ for: async () => [structuredClone(recorded(rowId))] }) }), + }), + update: () => ({ + set: (patch: Partial) => ({ + where: () => ({ + returning: async () => { + const value = recorded(rowId); + const json = operation_ledgers.canonical_result.mapToDriverValue( + patch.canonical_result ?? null + ); + Object.assign(value, { + ...patch, + canonical_result: operation_ledgers.canonical_result.mapFromDriverValue(json), + }); + return [structuredClone(value)]; + }, + }), + }), + }), + } as any; + } + function restart() { + rows = new Map(JSON.parse(JSON.stringify([...rows])) as [string, OperationLedgerRow][]); + for (const value of rows.values()) value.lease_expires_at = new Date(0).toISOString(); + jest.isolateModules(() => { + recoveredRun = jest.requireActual<{ + runBitbucketReviewOperation: typeof runBitbucketReviewOperation; + }>('./bitbucket-write').runBitbucketReviewOperation; + }); + } + function finishMerge() { + pr.state = 'MERGED'; + pr.merge_commit = { hash: 'c'.repeat(40) }; + } + beforeEach(() => { + acceptedTask = false; + taskComplete = false; + unavailable = false; + loseResponse = false; + dispatchSnapshots = []; + recoveredRun = runBitbucketReviewOperation; + const nativeRepository = { + uuid: `{${repository.repositoryId}}`, + full_name: repository.fullName, + workspace: { uuid: `{${repository.workspaceUuid}}` }, + }; + pr = { + type: 'pullrequest', + id: 7, + state: 'OPEN', + links: { html: { href: mergeRequest.intent.review.canonicalUrl } }, + source: { + repository: structuredClone(nativeRepository), + branch: { name: 'feature' }, + commit: { hash: mergeRequest.intent.revision.headSha }, + }, + destination: { + repository: structuredClone(nativeRepository), + branch: { name: 'trunk' }, + commit: { hash: mergeRequest.intent.revision.targetHeadSha }, + }, + }; + auth = { + userId, + repository, + authorization, + path: { + workspace: `{${repository.workspaceUuid}}`, + repo_slug: `{${repository.repositoryId}}`, + }, + actor: { + provider: 'bitbucket', + instanceUrl: repository.instanceUrl, + id: mergeRequest.intent.actorId, + login: 'reviewer', + displayName: null, + avatarUrl: null, + }, + credentialKind: 'bitbucketOAuth', + scopes: ['pullrequest:write'], + client: { + execute: async input => { + if (input.operation === 'merge') { + dispatchSnapshots = structuredClone( + [...rows.values()].map(value => value.canonical_result) + ); + effects.push('merge'); + if (loseResponse) { + finishMerge(); + throw new BitbucketInteractiveClientError('transport_failed'); + } + if (acceptedTask) + return { status: 202, data: null, location: taskUrl, metadata: {} } as any; + finishMerge(); + } else if (input.operation === 'mergeTask') { + if (unavailable) throw new BitbucketInteractiveClientError('temporarily_unavailable'); + if (taskComplete) finishMerge(); + return { + status: 200, + data: { + task_status: taskComplete ? 'SUCCESS' : 'PENDING', + links: { self: { href: taskUrl } }, + ...(taskComplete ? { merge_result: structuredClone(pr) } : {}), + }, + metadata: {}, + } as any; + } + return { status: 200, data: structuredClone(pr), metadata: {} } as any; + }, + }, + }; + jest.mocked(getBitbucketReview).mockImplementation( + async () => + ({ + identity: mergeRequest.intent.review, + revision: mergeRequest.intent.revision, + state: 'open', + source: { repository, branch: pr.source.branch.name }, + target: { repository, branch: pr.destination.branch.name }, + merge: { methods: [{ id: 'merge_commit', label: 'Merge commit' }] }, + authorization: { capabilities: reviewCapabilityFixtures('bitbucket') }, + }) as any + ); + jest + .mocked(recordOperationProgress) + .mockImplementation(async (_db, rowId, patch) => + actualLedger.recordOperationProgress(storage(rowId), rowId, patch) + ); + jest + .mocked(recordOperationAcceptance) + .mockImplementation(async (_db, input) => + actualLedger.recordOperationAcceptance(storage(input.rowId), input) + ); + }); + + it('AC7 stores server preflight identity before dispatch and ignores client evidence', async () => { + const forged = { + source: { ...evidence.source, branch: 'injected' }, + destination: evidence.destination, + }; + const result = await recoveredRun(auth, { + ...mergeRequest, + intent: { ...mergeRequest.intent, bitbucketMergeEvidence: forged }, + bitbucketMergeEvidence: forged, + } as BitbucketReviewOperationRequest); + expect(result).toEqual( + confirmedReviewEffect({ + provider: 'bitbucket', + kind: 'review', + id: '7', + url: mergeRequest.intent.review.canonicalUrl, + }) + ); + expect(dispatchSnapshots).toEqual([ + { result: unresolvedReviewEffect('dispatching'), bitbucketMergeEvidence: evidence }, + ]); + expect(row().canonical_result).toEqual({ result, bitbucketMergeEvidence: evidence }); + expect(Buffer.byteLength(JSON.stringify(row().canonical_result))).toBeLessThan(4096); + expect(effects).toEqual(['merge']); + }); + + it('AC7 retains the task and preflight identity through unavailable polling and process restart', async () => { + acceptedTask = true; + const accepted = await recoveredRun(auth, mergeRequest); + expect(accepted).toMatchObject({ status: 'accepted', reference: { id: 'task-1' } }); + restart(); + unavailable = true; + expect(await recoveredRun(auth, mergeRequest, true)).toMatchObject({ + status: 'unresolved', + retry: 'reconcile', + }); + expect(row().canonical_result).toEqual({ result: accepted, bitbucketMergeEvidence: evidence }); + expect(row().provider_ref).toBe( + JSON.stringify({ provider: 'bitbucket', kind: 'merge-task', id: 'task-1', url: taskUrl }) + ); + restart(); + unavailable = false; + taskComplete = true; + expect(await recoveredRun(auth, mergeRequest, true)).toMatchObject({ status: 'confirmed' }); + expect(await recoveredRun(auth, mergeRequest)).toMatchObject({ status: 'confirmed' }); + expect(row().status).toBe('completed'); + expect(row().canonical_result?.bitbucketMergeEvidence).toEqual(evidence); + expect(effects).toEqual(['merge']); + }); + + it('AC7 recovers a lost merge response from serialized preflight identity without another write', async () => { + loseResponse = true; + expect(await recoveredRun(auth, mergeRequest)).toMatchObject({ status: 'unresolved' }); + restart(); + loseResponse = false; + expect(await recoveredRun(auth, mergeRequest, true)).toMatchObject({ status: 'confirmed' }); + expect(pr.state).toBe('MERGED'); + expect(effects).toEqual(['merge']); + }); + + it.each( + (['source', 'destination'] as const).flatMap(endpoint => + (['branch', 'repository', 'workspace'] as const).map(field => ({ endpoint, field })) + ) + )( + 'AC7 rejects same-SHA $endpoint $field drift after serialized restart', + async ({ endpoint, field }) => { + loseResponse = true; + await recoveredRun(auth, mergeRequest); + restart(); + if (field === 'branch') pr[endpoint].branch.name = 'different'; + if (field === 'repository') + pr[endpoint].repository.uuid = '{77777777-7777-4777-8777-777777777777}'; + if (field === 'workspace') + pr[endpoint].repository.workspace.uuid = '{88888888-8888-4888-8888-888888888888}'; + expect(await recoveredRun(auth, mergeRequest, true)).toMatchObject({ + status: 'unresolved', + retry: 'reconcile', + }); + restart(); + expect(await recoveredRun(auth, mergeRequest)).toMatchObject({ status: 'unresolved' }); + expect(row().canonical_result?.bitbucketMergeEvidence).toEqual(evidence); + expect(effects).toEqual(['merge']); + } + ); + + it.each(['absent', 'malformed', 'incomplete', 'legacy accepted', 'legacy confirmed'] as const)( + 'AC7 never reconstructs %s merge identity from matching postflight objects', + async condition => { + acceptedTask = condition === 'legacy accepted'; + loseResponse = condition !== 'legacy accepted' && condition !== 'legacy confirmed'; + await recoveredRun(auth, mergeRequest); + const canonical = row().canonical_result!; + if (condition === 'malformed') + canonical.bitbucketMergeEvidence = { + ...evidence, + source: { ...evidence.source, repositoryId: 'not-a-uuid' }, + }; + else if (condition === 'incomplete') + canonical.bitbucketMergeEvidence = { source: evidence.source }; + else delete canonical.bitbucketMergeEvidence; + finishMerge(); + restart(); + expect(await recoveredRun(auth, mergeRequest, true)).toMatchObject({ + status: 'unresolved', + reason: 'merge_identity_unavailable', + retry: 'reconcile', + }); + restart(); + expect(await recoveredRun(auth, mergeRequest)).toMatchObject({ status: 'unresolved' }); + expect(effects).toEqual(['merge']); + } + ); + + it.each(['valid', 'absent', 'malformed', 'incomplete', 'missing child'] as const)( + 'AC7 checks serialized child identity beneath a cached aggregate: %s', + async condition => { + const aggregateRequest: BitbucketReviewOperationRequest = { + ...mergeRequest, + intent: { + ...mergeRequest.intent, + input: { + ...mergeRequest.intent.input, + deletion: { + effect: 'delete', + branch: 'feature', + expectedHeadSha: mergeRequest.intent.revision.headSha, + repositoryKey: repositoryResourceKey(userId, { repository, authorization }), + }, + }, + }, + }; + const execute = auth.client.execute; + auth.client.execute = async input => { + if (input.operation !== 'branch') return execute(input); + if (pr.state === 'MERGED') throw new BitbucketInteractiveClientError('not_found'); + return { + status: 200, + data: { name: 'feature', target: { hash: mergeRequest.intent.revision.headSha } }, + metadata: {}, + } as any; + }; + expect(await recoveredRun(auth, aggregateRequest)).toMatchObject({ status: 'confirmed' }); + const child = [...rows.values()].find( + value => value.intent === 'merge' && value.operation_key !== aggregateRequest.operationKey + )!; + expect(child.canonical_result).toMatchObject({ + result: { status: 'confirmed' }, + bitbucketMergeEvidence: evidence, + }); + const canonical = child.canonical_result!; + if (condition === 'missing child') rows.delete(rowKey(userId, child.operation_key)); + else if (condition === 'absent') delete canonical.bitbucketMergeEvidence; + else if (condition === 'malformed') + canonical.bitbucketMergeEvidence = { + ...evidence, + source: { ...evidence.source, repositoryId: 'not-a-uuid' }, + }; + else if (condition === 'incomplete') + canonical.bitbucketMergeEvidence = { source: evidence.source }; + + for (const statusOnly of [true, false]) { + restart(); + expect(await recoveredRun(auth, aggregateRequest, statusOnly)).toMatchObject( + condition === 'valid' + ? { status: 'confirmed', reference: { kind: 'review', id: '7' } } + : { + status: 'partial', + retry: 'unfinished-only', + items: [ + { + itemId: 'merge', + effect: 'merge', + result: + condition === 'missing child' + ? { status: 'rejected', code: 'operation_not_admitted', retry: 'same-key' } + : { + status: 'unresolved', + reason: 'merge_identity_unavailable', + retry: 'reconcile', + }, + }, + { + itemId: 'deleteBranch', + effect: 'deleteBranch', + result: { status: 'confirmed' }, + }, + ], + } + ); + expect(effects).toEqual(['merge']); + expect(rows.size).toBe(condition === 'missing child' ? 2 : 3); + } + } + ); + + it.each(['throw', 'no row'] as const)( + 'AC7 sends no merge when evidence persistence returns %s', + async failure => { + const progress = jest.mocked(recordOperationProgress).getMockImplementation()!; + jest.mocked(recordOperationProgress).mockImplementation(async (database, rowId, patch) => { + if (patch.bitbucketMergeEvidence) { + if (failure === 'throw') throw new Error('Storage unavailable'); + return null; + } + return progress(database, rowId, patch); + }); + expect(await recoveredRun(auth, mergeRequest)).toMatchObject({ + status: 'rejected', + code: 'preflight_unavailable', + retry: 'same-key', + }); + expect(effects).toEqual([]); + expect(pr.state).toBe('OPEN'); + jest.mocked(recordOperationProgress).mockImplementation(progress); + expect(await recoveredRun(auth, mergeRequest)).toMatchObject({ status: 'confirmed' }); + expect(effects).toEqual(['merge']); + } + ); + + it('AC7 rejects evidence that cannot fit the existing ledger before sending a merge', async () => { + pr.source.branch.name = 's'.repeat(2000); + pr.destination.branch.name = 'd'.repeat(2000); + expect(await recoveredRun(auth, mergeRequest)).toMatchObject({ + status: 'rejected', + code: 'preflight_unavailable', + retry: 'same-key', + }); + expect(effects).toEqual([]); + expect(pr.state).toBe('OPEN'); + expect(Buffer.byteLength(JSON.stringify(row().canonical_result))).toBeLessThan(4096); + }); +}); diff --git a/apps/web/src/lib/provider-review/operation.ts b/apps/web/src/lib/provider-review/operation.ts index adc284d7a0..bcfe57adfb 100644 --- a/apps/web/src/lib/provider-review/operation.ts +++ b/apps/web/src/lib/provider-review/operation.ts @@ -8,9 +8,11 @@ import { z } from 'zod'; import { CURRENT_UGC_TERMS_VERSION } from '@kilocode/app-shared/moderation'; import { PR_OPERATION_SETTLED_EVENT } from '@kilocode/app-shared/analytics'; import { + BitbucketMergeEvidenceSchema, ReviewEffectResultSchema, providerReviewIntentFingerprint, serializeReviewWriteRequest, + type BitbucketMergeEvidence, type ProviderReference, type ReviewAction, type ReviewIntent, @@ -28,6 +30,7 @@ import { operation_ledgers, user_terms_acceptances } from '@kilocode/db/schema'; import { db } from '@/lib/drizzle'; export type ReviewEffectResult = z.infer; +export type PersistBitbucketMergeEvidence = (evidence: BitbucketMergeEvidence) => Promise; export type ReviewOperationRequest = { userId: string; distinctId: string; @@ -81,8 +84,11 @@ export async function runReviewOperation( request: ReviewOperationRequest, handlers: { // Omission selects status-only reconciliation, never a new provider write. - execute?: () => Promise; - reconcile: (stored: ReviewEffectResult | null) => Promise; + execute?: (persistMergeEvidence?: PersistBitbucketMergeEvidence) => Promise; + reconcile: ( + stored: ReviewEffectResult | null, + mergeEvidence?: BitbucketMergeEvidence | null + ) => Promise; // Aggregate rows bind batches; only their individual provider effects emit outcomes. aggregate?: true; } @@ -157,6 +163,21 @@ export async function runReviewOperation( return rejectedReviewEffect('operation_key_reuse_mismatch'); const parsed = ReviewEffectResultSchema.safeParse(row.canonical_result?.result); const stored = parsed.success ? parsed.data : null; + const needsMergeEvidence = + intent.review.repository.provider === 'bitbucket' && action === 'merge' && !handlers.aggregate; + const parsedEvidence = BitbucketMergeEvidenceSchema.safeParse( + row.canonical_result?.bitbucketMergeEvidence + ); + let mergeEvidence: BitbucketMergeEvidence | null = + needsMergeEvidence && parsedEvidence.success ? parsedEvidence.data : null; + // Old Bitbucket rows lack preflight identity. Keep them unresolved until their retention expires; + // neither a saved result nor matching postflight heads can reconstruct the intended branches. + if ( + needsMergeEvidence && + !mergeEvidence && + (stored?.status === 'confirmed' || stored?.status === 'accepted') + ) + return unresolvedReviewEffect('merge_identity_unavailable', stored.reference); if (admission.admission === 'duplicate_settled') return stored && (stored.status === 'confirmed' || (stored.status === 'rejected' && stored.retry === 'never')) @@ -218,18 +239,43 @@ export async function runReviewOperation( (stored?.status === 'rejected' && stored.retry === 'same-key')) ) { // Retire a previous safe-retry receipt before dispatch. A lost response must not leave it replayable. + const dispatchResult = unresolvedReviewEffect('dispatching'); const dispatching = await recordOperationProgress(db, row.id, { - result: unresolvedReviewEffect('dispatching'), + result: dispatchResult, + ...(needsMergeEvidence ? { bitbucketMergeEvidence: null } : {}), }); if (!dispatching) throw new Error('Dispatch admission did not persist'); - result = await handlers.execute(); + mergeEvidence = null; + result = await handlers.execute(async evidence => { + if (!needsMergeEvidence) throw new Error('Unexpected merge evidence'); + // Only the server handler supplies evidence, after its authorized preflight and before dispatch. + const canonicalResult = { + result: dispatchResult, + bitbucketMergeEvidence: BitbucketMergeEvidenceSchema.parse(evidence), + }; + if ( + Buffer.byteLength(JSON.stringify(canonicalResult), 'utf8') >= MAX_CANONICAL_RESULT_BYTES + ) + throw new Error('Merge evidence exceeds the ledger limit'); + if (!(await recordOperationProgress(db, row.id, canonicalResult))) + throw new Error('Merge evidence did not persist'); + mergeEvidence = canonicalResult.bitbucketMergeEvidence; + }); } else result = stored?.status === 'rejected' && stored.retry === 'same-key' ? stored - : await handlers.reconcile(stored); + : await handlers.reconcile(stored, mergeEvidence); result = ReviewEffectResultSchema.parse(result); - if (Buffer.byteLength(JSON.stringify({ result }), 'utf8') >= MAX_CANONICAL_RESULT_BYTES) + if ( + Buffer.byteLength( + JSON.stringify({ + result, + ...(mergeEvidence ? { bitbucketMergeEvidence: mergeEvidence } : {}), + }), + 'utf8' + ) >= MAX_CANONICAL_RESULT_BYTES + ) result = unresolvedReviewEffect('result_too_large'); } catch { result = unresolvedReviewEffect('provider_outcome_unknown'); diff --git a/packages/app-shared/src/provider-review/contracts.test.ts b/packages/app-shared/src/provider-review/contracts.test.ts index b1d5803052..8f299e813a 100644 --- a/packages/app-shared/src/provider-review/contracts.test.ts +++ b/packages/app-shared/src/provider-review/contracts.test.ts @@ -2,6 +2,7 @@ import { describe, expect, expectTypeOf, it } from 'vitest'; import { CODE_REVIEW_PLATFORMS } from '../code-review/enums'; import { normalizeLegacyGitHubReviewRepository } from '../code-review/repository-identity'; import { + BitbucketMergeEvidenceSchema, BitbucketMergeTaskSchema, ProviderReferenceSchema, ProviderReviewStateSchema, @@ -363,3 +364,58 @@ describe('normalized provider review contracts', () => { expect(() => serializeReviewWriteRequest(undefined)).toThrow(); }); }); + +describe('persisted Bitbucket merge evidence', () => { + const destination = { + repositoryId: '11111111-1111-4111-8111-111111111111', + workspaceUuid: '22222222-2222-4222-8222-222222222222', + fullName: 'team/repo', + branch: 'trunk', + }; + const evidence = { + source: { + repositoryId: '33333333-3333-4333-8333-333333333333', + workspaceUuid: '44444444-4444-4444-8444-444444444444', + fullName: 'contributor/fork', + branch: 'feature/review', + }, + destination, + }; + it('retains distinct fork and destination identities after JSON storage', () => { + expect(BitbucketMergeEvidenceSchema.parse(JSON.parse(JSON.stringify(evidence)))).toEqual( + evidence + ); + expect( + ReviewMutationResultSchema.safeParse({ ...confirmed, bitbucketMergeEvidence: evidence }) + .success + ).toBe(false); + }); + it.each( + (['source', 'destination'] as const).flatMap(endpoint => + [ + { repositoryId: undefined }, + { repositoryId: 'team/repo' }, + { workspaceUuid: undefined }, + { workspaceUuid: 'team' }, + { fullName: '' }, + { branch: undefined }, + { branch: null }, + { branch: '' }, + { clientSupplied: true }, + ].map(change => ({ endpoint, change })) + ) + )('rejects malformed $endpoint evidence: $change', ({ endpoint, change }) => { + expect( + BitbucketMergeEvidenceSchema.safeParse({ + ...evidence, + [endpoint]: { ...evidence[endpoint], ...change }, + }).success + ).toBe(false); + }); + it.each([undefined, null, {}, { source: evidence.source }, { destination }])( + 'does not invent missing persisted identity: %j', + value => { + expect(BitbucketMergeEvidenceSchema.safeParse(value).success).toBe(false); + } + ); +}); diff --git a/packages/app-shared/src/provider-review/contracts.ts b/packages/app-shared/src/provider-review/contracts.ts index db7a82a7d1..ce51bbb0e7 100644 --- a/packages/app-shared/src/provider-review/contracts.ts +++ b/packages/app-shared/src/provider-review/contracts.ts @@ -213,6 +213,21 @@ export const BitbucketMergeTaskSchema = z.strictObject({ error: z.string().nullable(), }); export type BitbucketMergeTask = z.infer; + +const bitbucketMergeEndpoint = z.strictObject({ + repositoryId: z.uuid(), + workspaceUuid: z.uuid(), + fullName: id.max(511), + branch: id.max(4096), +}); +// Server-observed preflight evidence lives beside the ledger result, never in the write intent. +// Old ledger rows omit it until their 30-day retention expires; absence cannot confirm a merge. +export const BitbucketMergeEvidenceSchema = z.strictObject({ + source: bitbucketMergeEndpoint, + destination: bitbucketMergeEndpoint, +}); +export type BitbucketMergeEvidence = z.infer; + export const ReviewCheckSchema = z.strictObject({ id, name: id, diff --git a/packages/worker-utils/src/bitbucket-workspace-access-token.test.ts b/packages/worker-utils/src/bitbucket-workspace-access-token.test.ts index 9034cb0407..2c05285330 100644 --- a/packages/worker-utils/src/bitbucket-workspace-access-token.test.ts +++ b/packages/worker-utils/src/bitbucket-workspace-access-token.test.ts @@ -11,6 +11,7 @@ import { buildBitbucketOrganizationCredentialLockKey, buildBitbucketWorkspaceAccessTokenAad, getMissingBitbucketWorkspaceAccessTokenScopes, + getBitbucketReviewGrantStatus, getUnexpectedBitbucketWorkspaceAccessTokenScopes, hasBitbucketAccessTokenFamilyPrefix, hasRequiredBitbucketWorkspaceAccessTokenScopes, @@ -215,6 +216,40 @@ describe('Bitbucket Workspace Access Token contract', () => { ).toBe(false); }); + it.each([ + { method: 'oauth', recoveryAction: 'reconnect' }, + { method: 'workspace_access_token', recoveryAction: 'replace_token' }, + ] as const)( + 'keeps old $method grants readable and enables writes only after recovery', + ({ method, recoveryAction }) => { + const scopes = Object.freeze(['account', 'repository:write', 'pullrequest', 'webhook']); + expect(getBitbucketReviewGrantStatus(scopes, method)).toEqual({ + readReady: true, + writeReady: false, + recoveryAction, + }); + expect( + getBitbucketReviewGrantStatus(['account', 'pullrequest:write', 'webhook'], method) + ).toEqual({ + readReady: true, + writeReady: true, + recoveryAction: null, + }); + for (const incompleteScopes of [ + [], + ['pullrequest:write'], + ['account', 'pullrequest:write'], + ['pullrequest:write', 'webhook'], + ]) { + expect(getBitbucketReviewGrantStatus(incompleteScopes, method)).toEqual({ + readReady: false, + writeReady: false, + recoveryAction, + }); + } + } + ); + it('validates fixed-host repository pagination consistently', () => { expect( isValidBitbucketRepositoryPaginationUrl( diff --git a/packages/worker-utils/src/bitbucket-workspace-access-token.ts b/packages/worker-utils/src/bitbucket-workspace-access-token.ts index 2bb78def5e..c4f6b05ef1 100644 --- a/packages/worker-utils/src/bitbucket-workspace-access-token.ts +++ b/packages/worker-utils/src/bitbucket-workspace-access-token.ts @@ -238,3 +238,23 @@ export function hasRequiredBitbucketWorkspaceAccessTokenScopes( ): boolean { return getMissingBitbucketWorkspaceAccessTokenScopes(observedScopes).length === 0; } + +export function getBitbucketReviewGrantStatus( + observedScopes: readonly string[], + method: 'oauth' | 'workspace_access_token' +) { + // Old read grants remain valid until old clients/records and the 30-day ledger window expire. + const readReady = hasRequiredBitbucketWorkspaceAccessTokenScopes(observedScopes); + const writeReady = + readReady && + buildBitbucketWorkspaceAccessTokenEffectiveScopeSet(observedScopes).has('pullrequest:write'); + return { + readReady, + writeReady, + recoveryAction: writeReady + ? null + : method === 'oauth' + ? ('reconnect' as const) + : ('replace_token' as const), + }; +} diff --git a/services/git-token-service/src/bitbucket-authorization-service.test.ts b/services/git-token-service/src/bitbucket-authorization-service.test.ts index f2c0d43198..46262aa860 100644 --- a/services/git-token-service/src/bitbucket-authorization-service.test.ts +++ b/services/git-token-service/src/bitbucket-authorization-service.test.ts @@ -1,6 +1,7 @@ import { generateKeyPairSync } from 'node:crypto'; import type * as DbClientModule from '@kilocode/db/client'; -import { encryptKeyedEnvelope } from '@kilocode/encryption'; +import { decryptKeyedEnvelope, encryptKeyedEnvelope } from '@kilocode/encryption'; +import { getBitbucketReviewGrantStatus } from '@kilocode/worker-utils/bitbucket-workspace-access-token'; import { beforeEach, describe, expect, it, vi } from 'vitest'; const database = vi.hoisted(() => ({ @@ -41,12 +42,18 @@ vi.mock('@kilocode/db/client', async importOriginal => { update: () => ({ set: (values: Record) => { database.updates.push(values); + if (database.row && 'scopes' in values) database.row.scopes = values.scopes; const result = { returning: async () => { if (!database.returnedCredential) return []; + const updated = { + ...database.returnedCredential, + ...values, + credential_version: database.returnedCredential.credential_version, + }; const row = database.row as { credential?: Record } | undefined; - if (row) row.credential = database.returnedCredential; - return [database.returnedCredential]; + if (row) row.credential = updated; + return [updated]; }, then: (resolve: (value: unknown[]) => unknown) => Promise.resolve([]).then(resolve), }; @@ -381,6 +388,164 @@ describe('BitbucketAuthorizationService', () => { ); }); + function refreshResponse(scope: string) { + return Response.json({ + access_token: 'next-access-token', + refresh_token: 'next-refresh-token', + token_type: 'bearer', + expires_in: 7200, + scope, + }); + } + + it.each([ + { + name: 'explicit write grants', + scope: 'account repository:write pullrequest pullrequest:write webhook snippet', + writeReady: true, + }, + { + name: 'implied-only write grants', + scope: 'account pullrequest:write webhook', + writeReady: true, + }, + { + name: 'legacy write aliases', + scope: + 'read:account:bitbucket-legacy write:pullrequest:bitbucket-legacy admin:webhook:bitbucket-legacy', + writeReady: true, + }, + { + name: 'old read grants', + scope: 'account repository:write pullrequest webhook', + writeReady: false, + }, + { + name: 'old read aliases', + scope: + 'read:account:bitbucket-legacy write:repository:bitbucket-legacy read:pullrequest:bitbucket-legacy admin:webhook:bitbucket-legacy', + writeReady: false, + }, + ])( + 'AC1 preserves $name through rotation and subsequent authorization', + async ({ scope, writeReady }) => { + const row = activeRow(-1); + row.scopes.push('pullrequest:write'); + database.row = row; + database.returnedCredential = { ...credential(), credential_version: 2 }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(refreshResponse(scope))); + const authorizationService = service(); + + await expect( + authorizationService.getAuthorization({ userId: 'user-1' }) + ).resolves.toMatchObject({ + status: 'available', + token: 'next-access-token', + integrationId: row.integrationId, + workspace: row.metadata.workspace, + }); + expect(row.scopes).toEqual( + expect.arrayContaining([ + 'account', + 'email', + 'pullrequest', + 'repository', + 'repository:write', + 'webhook', + ]) + ); + expect(row.scopes).not.toContain('snippet'); + expect(getBitbucketReviewGrantStatus(row.scopes, 'oauth')).toEqual({ + readReady: true, + writeReady, + recoveryAction: writeReady ? null : 'reconnect', + }); + expect(row.credential.credential_version).toBe(2); + expect( + decryptKeyedEnvelope( + row.credential.refresh_token_encrypted, + scheme, + { active: { keyId: 'active', privateKeyPem } }, + aad('refresh') + ) + ).toBe('next-refresh-token'); + await expect( + authorizationService.getAuthorization({ userId: 'user-1' }) + ).resolves.toMatchObject({ + status: 'available', + token: 'next-access-token', + integrationId: row.integrationId, + workspace: row.metadata.workspace, + }); + expect(database.updates.filter(update => 'access_token_encrypted' in update)).toHaveLength(1); + } + ); + + it.each(['pullrequest:write', 'write:pullrequest:bitbucket-legacy'])( + 'AC1 accepts stored %s without requiring explicit implied read scopes', + async grant => { + database.row = { ...activeRow(), scopes: ['account', grant, 'webhook'] }; + await expect(service().getAuthorization({ userId: 'user-1' })).resolves.toMatchObject({ + status: 'available', + token: 'access-token', + workspace: { slug: 'acme' }, + }); + } + ); + + it.each([ + 'account pullrequest:write', + 'pullrequest:write webhook', + 'account repository:write webhook', + '', + ])('AC10 does not invent missing grants from refresh scope "%s"', async scope => { + const row = activeRow(-1); + database.row = row; + const previousCredential = row.credential; + const previousScopes = [...row.scopes]; + database.returnedCredential = { ...credential(), credential_version: 2 }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(refreshResponse(scope))); + + await expect(service().getAuthorization({ userId: 'user-1' })).resolves.toEqual({ + status: 'temporarily_unavailable', + }); + expect(row.credential).toEqual(previousCredential); + expect(row.scopes).toEqual(previousScopes); + expect(database.updates).toEqual([]); + }); + + it('AC1 retains credentials after a temporary refresh failure and permits retry', async () => { + const row = activeRow(-1); + database.row = row; + const previousCredential = row.credential; + database.returnedCredential = { ...credential(), credential_version: 2 }; + vi.stubGlobal( + 'fetch', + vi + .fn() + .mockRejectedValueOnce(new Error('Temporary network failure')) + .mockResolvedValueOnce(refreshResponse('account pullrequest:write webhook')) + ); + const authorizationService = service(); + + await expect(authorizationService.getAuthorization({ userId: 'user-1' })).resolves.toEqual({ + status: 'temporarily_unavailable', + }); + expect(row.credential).toEqual(previousCredential); + expect(database.updates).toEqual([]); + await expect( + authorizationService.getAuthorization({ userId: 'user-1' }) + ).resolves.toMatchObject({ + status: 'available', + token: 'next-access-token', + }); + expect(getBitbucketReviewGrantStatus(row.scopes, 'oauth')).toEqual({ + readReady: true, + writeReady: true, + recoveryAction: null, + }); + }); + it('marks terminal invalid_grant refresh failures as reconnect required', async () => { database.row = activeRow(-1); vi.stubGlobal( diff --git a/services/git-token-service/src/bitbucket-authorization-service.ts b/services/git-token-service/src/bitbucket-authorization-service.ts index 6b68b83ccf..021713fe87 100644 --- a/services/git-token-service/src/bitbucket-authorization-service.ts +++ b/services/git-token-service/src/bitbucket-authorization-service.ts @@ -101,6 +101,7 @@ const BITBUCKET_OAUTH_SCOPE_ALIASES: Record = { 'admin:webhook:bitbucket-legacy': ['webhook'], pullrequest: ['pullrequest'], 'read:pullrequest:bitbucket-legacy': ['pullrequest'], + 'write:pullrequest:bitbucket-legacy': ['pullrequest:write'], offline_access: [], }; @@ -120,6 +121,10 @@ function normalizedScopes(scope: string): string[] | null { } } + if (scopes.has('pullrequest:write')) { + scopes.add('pullrequest'); + scopes.add('repository:write'); + } if (scopes.has('repository:write')) scopes.add('repository'); if (scopes.has('account')) scopes.add('email'); const allowed = new Set([ @@ -128,6 +133,7 @@ function normalizedScopes(scope: string): string[] | null { 'repository', 'repository:write', 'pullrequest', + 'pullrequest:write', 'webhook', ]); if (