diff --git a/apps/web/src/lib/cloud-agent/provider-branch-listing.ts b/apps/web/src/lib/cloud-agent/provider-branch-listing.ts index 84d51fb9628..90c5e9a27e6 100644 --- a/apps/web/src/lib/cloud-agent/provider-branch-listing.ts +++ b/apps/web/src/lib/cloud-agent/provider-branch-listing.ts @@ -35,6 +35,8 @@ import { import { authorizeProject, GitLabReviewError, + gitlabReviewErrorToTrpcError, + gitlabReviewOwnerFor, type GitLabReviewOwner, } from '@/lib/provider-review/gitlab-authorization'; import { @@ -43,6 +45,7 @@ import { BitbucketReviewError, type BitbucketRepositoryAccess, } from '@/lib/provider-review/bitbucket-authorization'; +import { providerReviewErrorToTrpcError } from '@/lib/provider-review/error-mapping'; import { fetchPage, repositoryPathGuard, @@ -108,28 +111,6 @@ async function requireActiveIntegration(owner: Owner, platform: string, label: s return integration; } -/** - * Map a classified Bitbucket refusal onto the router error states. The - * message is the authorization layer's fixed copy — it never embeds a token - * or a workspace identity. - */ -function bitbucketErrorToTrpcError(error: BitbucketReviewError): TRPCError { - switch (error.kind) { - case 'not_found': - return new TRPCError({ code: 'NOT_FOUND', message: error.message }); - case 'forbidden': - return new TRPCError({ code: 'FORBIDDEN', message: error.message }); - case 'reconnect': - return new TRPCError({ code: 'PRECONDITION_FAILED', message: error.message }); - case 'bad_request': - return new TRPCError({ code: 'BAD_REQUEST', message: error.message }); - default: - // stale_head cannot come from a read; retryable surfaces as a - // retryable gateway failure, never as an empty branch list. - return new TRPCError({ code: 'BAD_GATEWAY', message: error.message }); - } -} - type GitHubBranchRef = { name: string; isDefault: boolean }; function toListing(branches: GitHubBranchRef[]): ProviderBranchListing { @@ -265,24 +246,6 @@ async function listGitHubBranches( } } -/** A refusal meaning "this GitLab review call cannot proceed" — router worded. */ -function gitlabErrorToTrpcError(error: GitLabReviewError): TRPCError { - switch (error.kind) { - case 'not_found': - return new TRPCError({ code: 'NOT_FOUND', message: error.message }); - case 'forbidden': - return new TRPCError({ code: 'FORBIDDEN', message: error.message }); - case 'reconnect': - return new TRPCError({ code: 'PRECONDITION_FAILED', message: error.message }); - case 'bad_request': - return new TRPCError({ code: 'BAD_REQUEST', message: error.message }); - default: - // stale_head cannot come from a read; retryable surfaces as a - // retryable gateway failure, never as an empty branch list. - return new TRPCError({ code: 'BAD_GATEWAY', message: error.message }); - } -} - /** * Map a GitLab branches HTTP refusal onto the router error states. Only the * status is carried over; the adapter's message stays out of the copy. @@ -344,10 +307,7 @@ async function listGitLabBranches( // authorization boundary the Bitbucket path enforces through // authorizeRepository. Without it, any project path the connected token // can reach would be listable. - const reviewOwner: GitLabReviewOwner = - owner.type === 'org' - ? { type: 'organization', organizationId: owner.id, userId: actor.userId } - : { type: 'user', userId: actor.userId }; + const reviewOwner = gitlabReviewOwnerFor(owner, actor.userId); try { const access = await authorizeProject(reviewOwner, repositoryFullName); const branches = await fetchGitLabBranches( @@ -357,7 +317,7 @@ async function listGitLabBranches( ); return toListing(branches.map(branch => ({ name: branch.name, isDefault: branch.default }))); } catch (error) { - if (error instanceof GitLabReviewError) throw gitlabErrorToTrpcError(error); + if (error instanceof GitLabReviewError) throw gitlabReviewErrorToTrpcError(error, reviewOwner); if (error instanceof GitLabBranchesFetchError) throw gitlabBranchesErrorToTrpcError(error, reviewOwner); throw error; @@ -425,7 +385,7 @@ async function listBitbucketBranches( repoSlug ); } catch (error) { - if (error instanceof BitbucketReviewError) throw bitbucketErrorToTrpcError(error); + if (error instanceof BitbucketReviewError) throw providerReviewErrorToTrpcError(error); throw error; } @@ -468,7 +428,7 @@ async function listBitbucketBranches( if (page === MAX_BRANCH_PAGES - 1) truncated = true; } } catch (error) { - if (error instanceof BitbucketReviewError) throw bitbucketErrorToTrpcError(error); + if (error instanceof BitbucketReviewError) throw providerReviewErrorToTrpcError(error); throw error; } diff --git a/apps/web/src/lib/code-reviews/manual-code-review-jobs.test.ts b/apps/web/src/lib/code-reviews/manual-code-review-jobs.test.ts index e9ee04e9914..20e0c1143a7 100644 --- a/apps/web/src/lib/code-reviews/manual-code-review-jobs.test.ts +++ b/apps/web/src/lib/code-reviews/manual-code-review-jobs.test.ts @@ -9,6 +9,27 @@ const mockGetAgentConfigForOwner = jest.fn(); const mockAssertCouncilCreationAllowed = jest.fn(); const mockCreateCodeReview = jest.fn(); const mockTryDispatchPendingReviews = jest.fn(); +const mockGetAllIntegrationsForOwner = jest.fn(); +const mockGetIntegrationForOwner = jest.fn(); +const mockGetValidGitLabToken = jest.fn(); +const mockFetchGitLabMergeRequest = jest.fn(); + +jest.mock('@/lib/integrations/db/platform-integrations', () => ({ + getAllIntegrationsForOwner: (...args: unknown[]) => mockGetAllIntegrationsForOwner(...args), + getIntegrationForOwner: (...args: unknown[]) => mockGetIntegrationForOwner(...args), +})); + +jest.mock('@/lib/integrations/gitlab-service', () => ({ + getValidGitLabToken: (...args: unknown[]) => mockGetValidGitLabToken(...args), +})); + +// The adapter's GitLab transport binds the resolved address and bypasses +// global fetch for self-managed hosts, so the connected path stubs the one +// provider read it makes and asserts the call it was given. +jest.mock('@/lib/integrations/platforms/gitlab/adapter', () => ({ + ...jest.requireActual('@/lib/integrations/platforms/gitlab/adapter'), + fetchGitLabMergeRequest: (...args: unknown[]) => mockFetchGitLabMergeRequest(...args), +})); jest.mock('@kilocode/web-shared/lib/config.server', () => ({ isLocalCodeReviewDevelopmentEnabled: () => mockIsLocalCodeReviewDevelopmentEnabled(), @@ -201,6 +222,188 @@ describe('createManualCodeReviewJob provider failures', () => { }); }); +describe('createManualCodeReviewJob connected GitLab source', () => { + const INSTANCE_URL = 'https://gitlab.example.com'; + const PROJECT_PATH = 'group/project'; + const MR_URL = `${INSTANCE_URL}/${PROJECT_PATH}/-/merge_requests/7`; + + const orgOwner = { type: 'org' as const, id: 'org-1', userId: 'bot-1' }; + + function gitlabIntegration() { + return { + id: 'gitlab-integration-1', + platform: 'gitlab', + integration_status: 'active', + owned_by_user_id: OWNER.id, + owned_by_organization_id: null, + repositories: [ + { id: 501, name: 'project', full_name: PROJECT_PATH, private: true }, + { id: 502, name: 'other', full_name: 'group/other', private: false }, + ], + metadata: { gitlab_instance_url: INSTANCE_URL }, + }; + } + + function mergeRequest(overrides: Record = {}) { + return { + iid: 7, + title: 'Harden the thing', + state: 'opened', + draft: false, + source_branch: 'feature', + target_branch: 'main', + sha: 'sha-7', + web_url: MR_URL, + author: { username: 'gl-user' }, + // The provider's ids differ from the cached repository id (501) so the + // assertion pins which id the review row stores. + project_id: 999, + target_project_id: 999, + ...overrides, + }; + } + + function gitlabJob(url: string) { + return { platform: 'gitlab' as const, url, modelSlug: 'test-model' }; + } + + beforeEach(() => { + mockIsLocalCodeReviewDevelopmentEnabled.mockReturnValue(false); + mockGetAllIntegrationsForOwner.mockResolvedValue([gitlabIntegration()]); + mockGetIntegrationForOwner.mockResolvedValue(gitlabIntegration()); + mockGetValidGitLabToken.mockResolvedValue('glpat-token'); + mockFetchGitLabMergeRequest.mockResolvedValue(mergeRequest()); + mockCreateCodeReview.mockResolvedValue('review-gitlab'); + }); + + it('starts a review for a project inside the connection repository scope', async () => { + await expect( + createManualCodeReviewJob({ owner: OWNER, input: gitlabJob(MR_URL) }) + ).resolves.toEqual({ reviewId: 'review-gitlab', outputMode: 'provider' }); + + expect(mockGetValidGitLabToken).toHaveBeenCalledWith(gitlabIntegration(), { + userId: OWNER.userId, + }); + expect(mockFetchGitLabMergeRequest).toHaveBeenCalledWith({ + accessToken: 'glpat-token', + projectId: PROJECT_PATH, + mrIid: 7, + instanceUrl: INSTANCE_URL, + }); + expect(mockCreateCodeReview).toHaveBeenCalledWith( + expect.objectContaining({ + platform: 'gitlab', + platformIntegrationId: 'gitlab-integration-1', + repoFullName: PROJECT_PATH, + platformProjectId: 501, + prNumber: 7, + prUrl: MR_URL, + prAuthor: 'gl-user', + baseRef: 'main', + headRef: 'refs/merge-requests/7/head', + headSha: 'sha-7', + }) + ); + expect(mockTryDispatchPendingReviews).toHaveBeenCalledWith(OWNER); + }); + + it('refuses a project outside the connection repository scope before any credential or provider call', async () => { + const error = await captureError( + gitlabJob(`${INSTANCE_URL}/other-org/private-project/-/merge_requests/1`) + ); + + expect(error).toBeInstanceOf(TRPCError); + expect((error as TRPCError).code).toBe('NOT_FOUND'); + // The refusal happens before the connection's credential is minted or a + // provider request is made: no token, no read, no review row, no dispatch. + expect(mockGetValidGitLabToken).not.toHaveBeenCalled(); + expect(mockFetchGitLabMergeRequest).not.toHaveBeenCalled(); + expect(mockCreateCodeReview).not.toHaveBeenCalled(); + expect(mockTryDispatchPendingReviews).not.toHaveBeenCalled(); + }); + + it('matches the cached project path case-insensitively and reads only the canonical path', async () => { + const shoutyUrl = `${INSTANCE_URL}/GROUP/Project/-/merge_requests/7`; + + await expect( + createManualCodeReviewJob({ owner: OWNER, input: gitlabJob(shoutyUrl) }) + ).resolves.toEqual({ reviewId: 'review-gitlab', outputMode: 'provider' }); + + expect(mockFetchGitLabMergeRequest).toHaveBeenCalledWith( + expect.objectContaining({ projectId: PROJECT_PATH }) + ); + expect(mockCreateCodeReview).toHaveBeenCalledWith( + expect.objectContaining({ repoFullName: PROJECT_PATH }) + ); + }); + + it('refuses a merge request URL on an instance other than the connected one', async () => { + const error = await captureError( + gitlabJob(`https://gitlab.com/${PROJECT_PATH}/-/merge_requests/7`) + ); + + expect(error).toBeInstanceOf(TRPCError); + expect((error as TRPCError).code).toBe('NOT_FOUND'); + expect(mockGetValidGitLabToken).not.toHaveBeenCalled(); + expect(mockFetchGitLabMergeRequest).not.toHaveBeenCalled(); + }); + + it('keeps the connect-GitLab precondition when the owner has no active GitLab connection', async () => { + mockGetAllIntegrationsForOwner.mockResolvedValue([]); + + const error = await captureError(gitlabJob(MR_URL)); + + expect(error).toBeInstanceOf(TRPCError); + expect((error as TRPCError).code).toBe('PRECONDITION_FAILED'); + expect((error as TRPCError).message).toContain('Connect GitLab'); + expect(mockGetIntegrationForOwner).not.toHaveBeenCalled(); + expect(mockGetValidGitLabToken).not.toHaveBeenCalled(); + }); + + it('maps a refused merge request read to a client error instead of a 500', async () => { + mockFetchGitLabMergeRequest.mockRejectedValue(new Error('GitLab MR fetch failed: 404')); + + const error = await captureError(gitlabJob(MR_URL)); + + expect(error).toBeInstanceOf(TRPCError); + expect((error as TRPCError).code).toBe('NOT_FOUND'); + expect(mockCreateCodeReview).not.toHaveBeenCalled(); + expect(mockTryDispatchPendingReviews).not.toHaveBeenCalled(); + }); + + it('maps a refused credential on the merge request read to the reconnect state', async () => { + mockFetchGitLabMergeRequest.mockRejectedValue(new Error('GitLab MR fetch failed: 401')); + + const error = await captureError(gitlabJob(MR_URL)); + + expect(error).toBeInstanceOf(TRPCError); + expect((error as TRPCError).code).toBe('PRECONDITION_FAILED'); + expect((error as TRPCError).message).toMatch(/reconnect/i); + expect(mockCreateCodeReview).not.toHaveBeenCalled(); + }); + + it('passes the organization credential actor for an organization-owned connection', async () => { + mockGetAllIntegrationsForOwner.mockResolvedValue([ + { ...gitlabIntegration(), owned_by_user_id: null, owned_by_organization_id: 'org-1' }, + ]); + mockGetIntegrationForOwner.mockResolvedValue({ + ...gitlabIntegration(), + owned_by_user_id: null, + owned_by_organization_id: 'org-1', + }); + + await expect( + createManualCodeReviewJob({ owner: orgOwner, input: gitlabJob(MR_URL) }) + ).resolves.toEqual({ reviewId: 'review-gitlab', outputMode: 'provider' }); + + expect(mockGetIntegrationForOwner).toHaveBeenCalledWith({ type: 'org', id: 'org-1' }, 'gitlab'); + expect(mockGetValidGitLabToken).toHaveBeenCalledWith(expect.anything(), { + userId: 'bot-1', + organizationId: 'org-1', + }); + }); +}); + describe('createManualCodeReviewJob happy path', () => { afterEach(() => { jest.restoreAllMocks(); diff --git a/apps/web/src/lib/code-reviews/manual-code-review-jobs.ts b/apps/web/src/lib/code-reviews/manual-code-review-jobs.ts index 90fd1cae887..0c2fc966002 100644 --- a/apps/web/src/lib/code-reviews/manual-code-review-jobs.ts +++ b/apps/web/src/lib/code-reviews/manual-code-review-jobs.ts @@ -2,7 +2,6 @@ import 'server-only'; import { TRPCError } from '@trpc/server'; import * as z from 'zod'; -import type { PlatformIntegration } from '@kilocode/db/schema'; import { CodeReviewCouncilConfigSchema, MAX_RUNTIME_AGENT_MODEL_LENGTH, @@ -27,9 +26,12 @@ import { PLATFORM } from '@/lib/integrations/core/constants'; import { getAllIntegrationsForOwner } from '@/lib/integrations/db/platform-integrations'; import { generateGitHubInstallationToken } from '@/lib/integrations/platforms/github/adapter'; import { getGitHubPullRequestCheckoutRef } from '@/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref'; -import { getValidGitLabToken } from '@/lib/integrations/gitlab-service'; import { fetchGitLabMergeRequest } from '@/lib/integrations/platforms/gitlab/adapter'; -import { normalizeGitLabInstanceUrl } from '@/lib/integrations/platforms/gitlab/instance-url'; +import { + authorizeProject, + gitlabReviewErrorToTrpcError, + gitlabReviewOwnerFor, +} from '@/lib/provider-review/gitlab-authorization'; import type { CodeReviewPlatform, Owner } from './core'; import { createCodeReview, findActiveProviderPublishingReview } from './db/code-reviews'; import { tryDispatchPendingReviews } from './dispatch/dispatch-pending-reviews'; @@ -524,38 +526,34 @@ async function resolveConnectedGitLabSource( }); } - for (const integration of integrations) { - const instanceUrl = getGitLabIntegrationInstanceUrl(integration); - if (parsed.origin !== instanceUrl) continue; + const reviewOwner = gitlabReviewOwnerFor(owner, owner.userId); - const accessToken = await getValidGitLabToken(integration, { - userId: owner.userId, - ...(owner.type === 'org' ? { organizationId: owner.id } : {}), - }); + try { + // The same boundary GitLab branch listing and the provider-review reads + // enforce: the pasted project has to be one of the connection's cached + // repositories, and the pasted instance has to be the connected one. A + // refusal here happens before the connection's credential is minted, so a + // path outside that scope never reaches the token broker, the provider, a + // review row, or session preparation. + const access = await authorizeProject(reviewOwner, parsed.projectPath, parsed.origin); const rawMergeRequest = await fetchGitLabMergeRequest({ - accessToken, - projectId: parsed.projectPath, + accessToken: access.accessToken, + projectId: access.projectPath, mrIid: parsed.mrIid, - instanceUrl, + instanceUrl: access.instanceUrl, }); const mergeRequest = GitLabMergeRequestApiSchema.parse(rawMergeRequest); validateOpenGitLabMergeRequest(mergeRequest); return buildGitLabSource({ mergeRequest, - projectPath: parsed.projectPath, - integrationId: integration.id, - platformProjectId: - mergeRequest.target_project_id ?? - mergeRequest.project_id ?? - getGitLabRepositoryIdFromIntegration(integration, parsed.projectPath), + projectPath: access.projectPath, + integrationId: access.integrationId, + platformProjectId: access.repositoryId, }); + } catch (error) { + throw gitlabReviewErrorToTrpcError(error, reviewOwner); } - - throw new TRPCError({ - code: 'BAD_REQUEST', - message: 'The merge request URL does not match a connected GitLab instance.', - }); } function buildGitHubSource( @@ -754,27 +752,6 @@ function validateOpenGitLabMergeRequest(mergeRequest: GitLabMergeRequestApi): vo } } -function getGitLabIntegrationInstanceUrl(integration: PlatformIntegration): string { - const metadata = integration.metadata; - const instanceUrl = - typeof metadata === 'object' && - metadata !== null && - !Array.isArray(metadata) && - 'gitlab_instance_url' in metadata - ? metadata.gitlab_instance_url - : undefined; - return normalizeGitLabInstanceUrl(typeof instanceUrl === 'string' ? instanceUrl : undefined); -} - -function getGitLabRepositoryIdFromIntegration( - integration: PlatformIntegration, - projectPath: string -): number | undefined { - const repositories = integration.repositories; - const repository = repositories?.find(candidate => candidate.full_name === projectPath); - return typeof repository?.id === 'number' ? repository.id : undefined; -} - class ProviderFetchError extends Error { readonly status: number; diff --git a/apps/web/src/lib/provider-review/error-mapping.ts b/apps/web/src/lib/provider-review/error-mapping.ts new file mode 100644 index 00000000000..fe554106c11 --- /dev/null +++ b/apps/web/src/lib/provider-review/error-mapping.ts @@ -0,0 +1,33 @@ +/** + * One classified provider refusal → the router's error state. + * + * Every provider surface (the mobile/web provider-review router, the Cloud + * Agent branch listing, and the manual Code Reviewer job) answers the same + * refusal class with the same code, so a client that handles one surface + * handles them all. The provider message is fixed copy that never embeds a + * token, an instance URL, or a workspace identity. + */ +import 'server-only'; + +import { TRPCError } from '@trpc/server'; +import type { ProviderReviewError } from '@kilocode/app-shared/provider-review'; + +export function providerReviewErrorToTrpcError(error: ProviderReviewError): TRPCError { + switch (error.kind) { + case 'not_found': + return new TRPCError({ code: 'NOT_FOUND', message: error.message, cause: error }); + case 'forbidden': + return new TRPCError({ code: 'FORBIDDEN', message: error.message, cause: error }); + case 'reconnect': + // The provider connection must be re-established; the caller's Kilo + // access is not the problem. This is the mobile reconnect state. + return new TRPCError({ code: 'PRECONDITION_FAILED', message: error.message, cause: error }); + case 'stale_head': + return new TRPCError({ code: 'CONFLICT', message: error.message, cause: error }); + case 'bad_request': + return new TRPCError({ code: 'BAD_REQUEST', message: error.message, cause: error }); + case 'retryable': + // A provider-side or network failure to retry, never a 500. + return new TRPCError({ code: 'BAD_GATEWAY', message: error.message, cause: error }); + } +} diff --git a/apps/web/src/lib/provider-review/gitlab-authorization.test.ts b/apps/web/src/lib/provider-review/gitlab-authorization.test.ts index 5ba11cb14a0..81f8629bdac 100644 --- a/apps/web/src/lib/provider-review/gitlab-authorization.test.ts +++ b/apps/web/src/lib/provider-review/gitlab-authorization.test.ts @@ -10,6 +10,7 @@ import { classifyGitLabStatus, GitLabApiStatusError, GitLabReviewError, + gitlabReviewErrorToTrpcError, type GitLabReviewOwner, } from './gitlab-authorization'; @@ -181,6 +182,9 @@ describe('authorizeProject — repository matching', () => { const access = await authorizeProject(SELF_MANAGED_USER, 'GROUP/Sub/REPO'); expect(access.projectPath).toBe('group/sub/repo'); + // The resolved identity travels with the credentials so a caller can + // record which connection and repository an action belongs to. + expect(access).toMatchObject({ integrationId: 'intg_1', repositoryId: 7 }); }); it('refuses a prefix or suffix of a nested path (exact full path only)', async () => { @@ -347,3 +351,40 @@ describe('classifyGitLabErrorForOwner', () => { ); }); }); + +describe('gitlabReviewErrorToTrpcError', () => { + it.each([ + ['not_found', 'NOT_FOUND'], + ['forbidden', 'FORBIDDEN'], + ['reconnect', 'PRECONDITION_FAILED'], + ['stale_head', 'CONFLICT'], + ['bad_request', 'BAD_REQUEST'], + ['retryable', 'BAD_GATEWAY'], + ] as const)('maps the %s kind to %s', (kind, code) => { + const mapped = gitlabReviewErrorToTrpcError( + new GitLabReviewError(kind, 'fixed copy'), + SELF_MANAGED_USER + ); + + expect(mapped.code).toBe(code); + expect(mapped.message).toBe('fixed copy'); + }); + + it('classifies before mapping, so an organization 403 is a reconnect', () => { + const refusal = new GitLabReviewError('forbidden', 'denied', 403); + + expect(gitlabReviewErrorToTrpcError(refusal, SELF_MANAGED_ORG).code).toBe( + 'PRECONDITION_FAILED' + ); + expect(gitlabReviewErrorToTrpcError(refusal, SELF_MANAGED_USER).code).toBe('FORBIDDEN'); + }); + + it('passes an intentional tRPC refusal through unchanged', () => { + const refusal = new TRPCError({ + code: 'BAD_REQUEST', + message: 'Draft merge requests are skipped.', + }); + + expect(gitlabReviewErrorToTrpcError(refusal, SELF_MANAGED_USER)).toBe(refusal); + }); +}); diff --git a/apps/web/src/lib/provider-review/gitlab-authorization.ts b/apps/web/src/lib/provider-review/gitlab-authorization.ts index 24a9a1c5491..10496bf8d7a 100644 --- a/apps/web/src/lib/provider-review/gitlab-authorization.ts +++ b/apps/web/src/lib/provider-review/gitlab-authorization.ts @@ -29,6 +29,7 @@ import { normalizeGitLabInstanceUrl, } from '@/lib/integrations/platforms/gitlab/instance-url'; import { logExceptInTest } from '@kilocode/web-shared/lib/utils.server'; +import { providerReviewErrorToTrpcError } from './error-mapping'; /** * The account that owns the GitLab integration. `userId` is the acting user @@ -212,9 +213,37 @@ export type GitLabProjectAccess = { instanceUrl: string; /** The integration's repository full path, matched case-insensitively. */ projectPath: string; + /** The integration the credential belongs to. */ + integrationId: string; + /** The matched repository's provider id from that integration's cache. */ + repositoryId: number; owner: GitLabReviewOwner; }; +/** + * Map one GitLab failure onto the router's error states. Classification is + * owner-aware first (a connection-level 403 is a reconnect, not the reader's + * permission), then every surface answers the kind with the shared code. A + * TRPCError is already an intentional refusal (invalid URL, closed or draft + * merge request) and passes through unchanged. + */ +export function gitlabReviewErrorToTrpcError(error: unknown, owner: GitLabReviewOwner): TRPCError { + if (error instanceof TRPCError) return error; + return providerReviewErrorToTrpcError(classifyGitLabErrorForOwner(error, owner)); +} + +/** + * The review-layer owner for a Kilo owner plus the acting user whose + * credential the token broker releases. Organization-owned connections keep + * the organization as the owner and name the acting user separately, so the + * broker can release the organization's credential for that user. + */ +export function gitlabReviewOwnerFor(owner: Owner, actingUserId: string): GitLabReviewOwner { + return owner.type === 'org' + ? { type: 'organization', organizationId: owner.id, userId: actingUserId } + : { type: 'user', userId: actingUserId }; +} + function ownerToDbOwner(owner: GitLabReviewOwner): Owner { return owner.type === 'user' ? { type: 'user', id: owner.userId } @@ -326,15 +355,24 @@ export async function authorizeProject( repo => repo.full_name.toLowerCase() === requested.toLowerCase() ); if (!match) { + // The repository list is the project's authorization scope, so this refusal + // names the one remedy that restores it. throw new GitLabReviewError( 'not_found', - 'This project is not part of your connected GitLab repositories.' + 'This project is not in your connected GitLab repositories. Refresh the repository list on the GitLab integration and try again.' ); } try { const accessToken = await getValidGitLabToken(integration, actorFor(owner)); - return { accessToken, instanceUrl, projectPath: match.full_name, owner }; + return { + accessToken, + instanceUrl, + projectPath: match.full_name, + integrationId: integration.id, + repositoryId: match.id, + owner, + }; } catch (error) { throw classifyGitLabErrorForOwner(error, owner); } diff --git a/apps/web/src/routers/provider-review-router.ts b/apps/web/src/routers/provider-review-router.ts index eaace91cd4d..eb112ea8cfa 100644 --- a/apps/web/src/routers/provider-review-router.ts +++ b/apps/web/src/routers/provider-review-router.ts @@ -61,6 +61,7 @@ import { unresolveThread as gitlabUnresolveThread, } from '@/lib/provider-review/gitlab-write'; import * as bitbucketRead from '@/lib/provider-review/bitbucket-read'; +import { providerReviewErrorToTrpcError } from '@/lib/provider-review/error-mapping'; import { BITBUCKET_AUTO_MERGE_UNSUPPORTED_REASON, BITBUCKET_PR_REVIEW_CAPABILITIES, @@ -383,24 +384,7 @@ function providerRef(input: ProviderRefIdentity): ProviderPrRef { */ function toProviderTrpcError(error: unknown): TRPCError { if (error instanceof TRPCError) return error; - if (error instanceof ProviderReviewError) { - switch (error.kind) { - case 'not_found': - return new TRPCError({ code: 'NOT_FOUND', message: error.message }); - case 'forbidden': - return new TRPCError({ code: 'FORBIDDEN', message: error.message }); - case 'reconnect': - // The provider connection must be re-established; the caller's Kilo - // access is not the problem. This is the mobile reconnect state. - return new TRPCError({ code: 'PRECONDITION_FAILED', message: error.message }); - case 'stale_head': - return new TRPCError({ code: 'CONFLICT', message: error.message }); - case 'bad_request': - return new TRPCError({ code: 'BAD_REQUEST', message: error.message }); - case 'retryable': - return new TRPCError({ code: 'BAD_GATEWAY', message: error.message }); - } - } + if (error instanceof ProviderReviewError) return providerReviewErrorToTrpcError(error); return new TRPCError({ code: 'INTERNAL_SERVER_ERROR', message: 'The review request failed. Please try again.',