From 4b7b9f1f6ef8a2612d9b7af4865231a1f49a4d04 Mon Sep 17 00:00:00 2001 From: "kiloconnect[bot]" <240665456+kiloconnect[bot]@users.noreply.github.com> Date: Tue, 29 Sep 2026 08:10:50 +0000 Subject: [PATCH] (janitor/comments): remove noise comments from platform integrations Delete restating JSDoc, section banners, numbered step narration, and duplicated @param boilerplate from the GitHub/GitLab platform adapters and webhook handlers. Comments carrying real constraints - external API behavior, fail-open choices, ledger/authorization invariants, @see references, lint directives - are kept. --- .../integrations/platforms/github/adapter.ts | 118 +------- .../platforms/github/app-selector.ts | 10 - .../platforms/github/webhook-handler.ts | 32 --- .../github/webhook-handlers/index.ts | 5 - .../installation-repositories-handler.ts | 5 - .../github/webhook-handlers/issue-handler.ts | 29 -- .../pull-request-handler.test.ts | 4 +- .../webhook-handlers/pull-request-handler.ts | 42 +-- .../upsert-cli-session-pull-requests.test.ts | 1 - .../platforms/github/webhook-helpers.ts | 10 - .../platforms/github/webhook-schemas.ts | 18 -- .../integrations/platforms/gitlab/adapter.ts | 261 +----------------- .../gitlab/webhook-handlers/index.ts | 6 - .../webhook-handlers/merge-request-handler.ts | 45 +-- .../platforms/gitlab/webhook-schemas.ts | 33 --- .../platforms/gitlab/webhook-sync.ts | 64 ----- 16 files changed, 38 insertions(+), 645 deletions(-) diff --git a/apps/web/src/lib/integrations/platforms/github/adapter.ts b/apps/web/src/lib/integrations/platforms/github/adapter.ts index 8a564e28e8..ac4fa084bb 100644 --- a/apps/web/src/lib/integrations/platforms/github/adapter.ts +++ b/apps/web/src/lib/integrations/platforms/github/adapter.ts @@ -41,10 +41,6 @@ export async function fetchGitHubInstallationRequests( })); } -/** - * Verifies GitHub webhook signature - * @param appType - The type of GitHub App to verify against (defaults to 'standard') - */ export function verifyGitHubWebhookSignature( payload: string, signature: string, @@ -61,10 +57,6 @@ export function verifyGitHubWebhookSignature( } } -/** - * Generates GitHub App installation token - * @param appType - The type of GitHub App to use (defaults to 'standard') - */ export async function generateGitHubInstallationToken( installationId: string, appType: GitHubAppType = 'standard', @@ -104,10 +96,6 @@ export async function generateGitHubInstallationTokenForMaintenance( }; } -/** - * Deletes a GitHub App installation - * @param appType - The type of GitHub App to use (defaults to 'standard') - */ export async function deleteGitHubInstallation( installationId: string, appType: GitHubAppType = 'standard' @@ -118,7 +106,6 @@ export async function deleteGitHubInstallation( throw new Error(`GitHub ${appType} App credentials not configured`); } - // Create app-level authentication (not installation-level) const auth = createAppAuth({ appId: credentials.appId, privateKey: credentials.privateKey, @@ -127,7 +114,6 @@ export async function deleteGitHubInstallation( const { token } = await auth({ type: 'app' }); const octokit = new Octokit({ auth: token }); - // Delete the installation await octokit.apps.deleteInstallation({ installation_id: parseInt(installationId), }); @@ -194,10 +180,6 @@ type GitHubBranch = { isDefault: boolean; }; -/** - * Fetches all repositories accessible by a GitHub App installation - * @param appType - The type of GitHub App to use (defaults to 'standard') - */ export async function fetchGitHubRepositories( installationId: string, appType: GitHubAppType = 'standard', @@ -212,7 +194,6 @@ export async function fetchGitHubRepositories( ); const octokit = new Octokit({ auth: tokenData.token }); - // Fetch all repositories accessible by the installation using pagination const repositories: GitHubRepository[] = []; let page = 1; const perPage = 100; @@ -223,7 +204,6 @@ export async function fetchGitHubRepositories( page, }); - // Filter out archived repositories repositories.push( ...data.repositories .filter(repo => !repo.archived) @@ -270,10 +250,6 @@ export async function fetchGitHubRepositoriesForMaintenance( return repositories; } -/** - * Fetches all branches for a GitHub repository - * @param appType - The type of GitHub App to use (defaults to 'standard') - */ export async function fetchGitHubBranches( installationId: string, repositoryFullName: string, @@ -291,14 +267,12 @@ export async function fetchGitHubBranches( const [owner, repo] = repositoryFullName.split('/'); - // Fetch the repository to get the default branch const { data: repoData } = await octokit.repos.get({ owner, repo, }); const defaultBranch = repoData.default_branch; - // Fetch all branches using pagination const branches: GitHubBranch[] = []; let page = 1; const perPage = 100; @@ -325,11 +299,6 @@ export async function fetchGitHubBranches( return branches; } -/* - * Fetches GitHub App installation details including permissions - * Uses app-level authentication to get installation info - * @param appType - The type of GitHub App to use (defaults to 'standard') - */ export async function fetchGitHubInstallationDetails( installationId: string, appType: GitHubAppType = 'standard' @@ -351,7 +320,6 @@ export async function fetchGitHubInstallationDetails( throw new Error(`GitHub ${appType} App credentials not configured`); } - // Create app-level authentication (not installation-level) const auth = createAppAuth({ appId: credentials.appId, privateKey: credentials.privateKey, @@ -379,9 +347,7 @@ export async function fetchGitHubInstallationDetails( } /** - * Adds a reaction to a PR (or issue) - * Used to show that Kilo is reviewing a PR (e.g., 👀 eyes reaction) - * @param appType - The type of GitHub App to use (defaults to 'standard') + * Used to show that Kilo is reviewing a PR (e.g., 👀 eyes reaction). */ export async function addReactionToPR( installationId: string, @@ -451,9 +417,7 @@ export async function hasPRCommentWithMarker( } /** - * Adds a reaction to a PR review comment - * Used to acknowledge @kilo fix mentions on inline review comments - * @param appType - The type of GitHub App to use (defaults to 'standard') + * Used to acknowledge @kilo fix mentions on inline review comments. */ export async function addReactionToPRReviewComment( installationId: string, @@ -475,10 +439,8 @@ export async function addReactionToPRReviewComment( } /** - * Checks the collaborator permission level for a user on a repository. * Returns the permission string ('admin' | 'write' | 'read' | 'none') or null * if the lookup fails (e.g. the App lacks permission to query collaborators). - * @param appType - The type of GitHub App to use (defaults to 'standard') */ export type CollaboratorPermission = 'admin' | 'write' | 'read' | 'none'; @@ -511,11 +473,6 @@ export async function getCollaboratorPermissionLevel( } } -/** - * Replies to a PR review comment thread - * Used by auto-fix to post completion/failure replies on review threads - * @param appType - The type of GitHub App to use (defaults to 'standard') - */ export async function replyToReviewComment( installationId: string, owner: string, @@ -546,9 +503,8 @@ const GitHubOAuthTokenResponseSchema = z.object({ }); /** - * Exchange GitHub OAuth code for user information - * Used during installation request flow to identify the GitHub user - * @param appType - The type of GitHub App to use (defaults to 'standard') + * Used during installation request flow to identify the GitHub user. + * * @param codeVerifier - The PKCE code verifier, required when the * authorization request that produced `code` included a code_challenge * (as `beginConnection` does). GitHub rejects redemption of such a code @@ -633,11 +589,8 @@ const KILO_REVIEW_COMMENTS_PER_PAGE = 100; const MAX_KILO_REVIEW_COMMENT_PAGES = 5; /** - * Finds an existing Kilo review comment on a PR - * Looks for the marker in issue comments - * Falls back to detecting older Kilo comments by patterns if no marker found - * Returns the most recent comment ID and body if found, null otherwise - * @param appType - The type of GitHub App to use (defaults to 'standard') + * Looks for the marker in issue comments. + * Falls back to detecting older Kilo comments by patterns if no marker found. */ export async function findKiloReviewComment( installationId: string, @@ -673,11 +626,9 @@ export async function findKiloReviewComment( totalComments: comments.length, }); - // Primary: Look for comments with the kilo-review marker const markedComments = comments.filter(c => c.body?.includes('')); if (markedComments.length > 0) { - // Sort by updated_at descending and pick the latest const latestComment = markedComments.sort((a, b) => { return new Date(b.updated_at).getTime() - new Date(a.updated_at).getTime(); })[0]; @@ -706,10 +657,6 @@ export async function findKiloReviewComment( return null; } -/** - * Updates an existing Kilo review comment on a GitHub PR - * Used to append usage footer (model + token count) after review completion - */ export async function updateKiloReviewComment( installationId: string, owner: string, @@ -736,9 +683,7 @@ export async function updateKiloReviewComment( } /** - * Fetches existing inline review comments on a PR - * Used to detect duplicates and track outdated comments - * @param appType - The type of GitHub App to use (defaults to 'standard') + * Used to detect duplicates and track outdated inline comments. */ export async function fetchPRInlineComments( installationId: string, @@ -805,11 +750,6 @@ export async function fetchPRInlineComments( return comments; } -/** - * Gets the HEAD commit SHA for a PR - * Required for creating inline comments via gh api - * @param appType - The type of GitHub App to use (defaults to 'standard') - */ export async function getPRHeadCommit( installationId: string, owner: string, @@ -1042,9 +982,6 @@ export async function getGitHubReviewComment( } } -/** - * Type guard to check if an error is an HTTP error from Octokit - */ function isHttpError(error: unknown): error is { status: number; message: string } { return ( typeof error === 'object' && @@ -1278,15 +1215,6 @@ export async function fetchPullRequestReviewDecision(args: { return results.get('pr0') ?? null; } -/** - * Get repository details including whether it's empty. - * Used to validate target repo before migration. - * - * @param installationId - The GitHub App installation ID - * @param repoFullName - The full name of the repository (owner/repo) - * @param appType - The type of GitHub App to use (defaults to 'standard') - * @returns Repository details or null if not found/not accessible - */ export async function getRepositoryDetails( installationId: string, repoFullName: string, @@ -1312,8 +1240,6 @@ export async function getRepositoryDetails( repo, }); - // Check if repo is empty by trying to get commits - // An empty repo has no commits let isEmpty = false; try { const { data: commits } = await octokit.repos.listCommits({ @@ -1345,7 +1271,6 @@ export async function getRepositoryDetails( isPrivate: repoData.private, }; } catch (error) { - // 404 means repo doesn't exist or not accessible if (isHttpError(error) && error.status === 404) { return null; } @@ -1354,12 +1279,8 @@ export async function getRepositoryDetails( } /** - * Get the URL to the GitHub App installation settings page. - * Users may need to grant access to newly created repos here. - * - * @param installationId - The GitHub App installation ID - * @param appType - The type of GitHub App to use (defaults to 'standard') - * @returns The URL to the installation settings page + * The installation settings page is where users grant access to newly + * created repos. */ export async function getInstallationSettingsUrl( installationId: string, @@ -1371,7 +1292,6 @@ export async function getInstallationSettingsUrl( throw new Error(`GitHub ${appType} App credentials not configured`); } - // Create app-level authentication to get installation details const auth = createAppAuth({ appId: credentials.appId, privateKey: credentials.privateKey, @@ -1384,24 +1304,15 @@ export async function getInstallationSettingsUrl( installation_id: parseInt(installationId), }); - // The account type determines the URL format const accountLogin = (data.account as { login?: string })?.login ?? ''; const accountType = (data.account as { type?: string })?.type ?? 'User'; - // GitHub App installation settings URL format - // For orgs: https://github.com/organizations/{org}/settings/installations/{id} - // For users: https://github.com/settings/installations/{id} if (accountType === 'Organization') { return `https://github.com/organizations/${accountLogin}/settings/installations/${installationId}`; } return `https://github.com/settings/installations/${installationId}`; } -/** - * Check if user already has a fork of a repository - * @param accountLogin - The GitHub username of the account where the fork would be created - * @param appType - The type of GitHub App to use (defaults to 'standard') - */ export async function checkExistingFork( installationId: string, accountLogin: string, @@ -1413,13 +1324,11 @@ export async function checkExistingFork( const octokit = new Octokit({ auth: tokenData.token }); try { - // Check if the user has a repo with the same name as the source const { data: repo } = await octokit.repos.get({ owner: accountLogin, repo: sourceRepo, }); - // Verify it's actually a fork of the source repo if (repo.fork && repo.parent?.full_name === `${sourceOwner}/${sourceRepo}`) { return { exists: true, @@ -1431,7 +1340,6 @@ export async function checkExistingFork( // This is an edge case - the fork will be created with a different name return { exists: false, fullName: null }; } catch (error) { - // 404 means the repo doesn't exist - no existing fork if (isHttpError(error) && error.status === 404) { return { exists: false, fullName: null }; } @@ -1439,10 +1347,6 @@ export async function checkExistingFork( } } -// ============================================================================ -// Commit Inspection -// ============================================================================ - /** * Checks whether a commit is a merge commit (has 2+ parents). * Used to skip code reviews triggered by "merge base into feature" pushes. @@ -1485,10 +1389,6 @@ export async function isMergeCommit( } } -// ============================================================================ -// Check Runs API (PR gate checks) -// ============================================================================ - /** * Conclusion values for a completed GitHub Check Run. * @see https://docs.github.com/en/rest/checks/runs#create-a-check-run diff --git a/apps/web/src/lib/integrations/platforms/github/app-selector.ts b/apps/web/src/lib/integrations/platforms/github/app-selector.ts index 56b21994ab..cd6bce86a8 100644 --- a/apps/web/src/lib/integrations/platforms/github/app-selector.ts +++ b/apps/web/src/lib/integrations/platforms/github/app-selector.ts @@ -9,9 +9,6 @@ import { Octokit } from '@octokit/rest'; */ export type GitHubAppType = 'standard' | 'lite'; -/** - * Credentials for a GitHub App - */ export type GitHubAppCredentials = { appId: string; privateKey: string; @@ -39,16 +36,9 @@ export async function getGitHubAppTypeForOrganization( return 'standard'; } - // Use the github_app_type from organization settings return organization.settings?.github_app_type ?? 'standard'; } -/** - * Gets the credentials for the specified GitHub App type. - * - * @param appType - The type of app to get credentials for - * @returns The credentials for the specified app type - */ export function getGitHubAppCredentials(appType: GitHubAppType): GitHubAppCredentials { if (appType === 'lite') { return { diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handler.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handler.ts index 4c0f13d3c2..49d4297ecd 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handler.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handler.ts @@ -96,26 +96,10 @@ async function isAvailableForDeferredGitHubDispatch(integration: { } } -/** - * Shared GitHub App Webhook Handler - * - * Handles webhooks for both standard and lite GitHub Apps. - * Thin routing layer that: - * 1. Verifies webhook signature - * 2. Parses the event - * 3. Routes to appropriate handler - * 4. Handles errors - * - * All business logic is in handler files - * - * @param request - The incoming Next.js request - * @param appType - 'standard' for full-featured app, 'lite' for read-only OSS app - */ export async function handleGitHubWebhook( request: NextRequest, appType: GitHubAppType ): Promise { - // Helper for app-specific logging const logSuffix = appType === 'lite' ? ' (lite app)' : ''; const sentryPrefix = appType === 'lite' ? 'github_lite_' : 'github_'; @@ -128,7 +112,6 @@ export async function handleGitHubWebhook( let deliveryAction: string | undefined; try { - // 1. Verify signature const rawBody = await request.text(); const signature = request.headers.get('x-hub-signature-256') || ''; @@ -137,7 +120,6 @@ export async function handleGitHubWebhook( return new NextResponse('Unauthorized', { status: 401 }); } - // 2. Parse JSON payload let payload: unknown; try { payload = JSON.parse(rawBody); @@ -150,7 +132,6 @@ export async function handleGitHubWebhook( return NextResponse.json({ error: 'Invalid JSON payload' }, { status: 400 }); } - // 3. Get event type and action from headers eventType = request.headers.get('x-github-event') || ''; const eventSignature = request.headers.get('x-github-delivery'); deliveryId = eventSignature; @@ -181,13 +162,11 @@ export async function handleGitHubWebhook( ); } - // 4. Helper function to log webhook events const logWebhook = async ( integration: { owned_by_organization_id: string | null; owned_by_user_id: string | null }, action: string ) => { try { - // Determine owner from integration const owner = integration.owned_by_organization_id ? { type: 'org' as const, id: integration.owned_by_organization_id } : ({ type: 'user' as const, id: integration.owned_by_user_id } as Owner); @@ -258,8 +237,6 @@ export async function handleGitHubWebhook( return response; }; - // 5. Route based on event type with type-safe Zod parsing - if (eventType === GITHUB_EVENT.APP_AUTHORIZATION) { const parseResult = GitHubAppAuthorizationRevokedPayloadSchema.safeParse(payload); if (!parseResult.success) { @@ -273,7 +250,6 @@ export async function handleGitHubWebhook( return NextResponse.json({ message: 'Authorization revoked' }, { status: 200 }); } - // Handle installation events if (eventType === GITHUB_EVENT.INSTALLATION) { const action = (payload as { action?: string }).action || ''; @@ -369,7 +345,6 @@ export async function handleGitHubWebhook( const result = await handleInstallationSuspend(parseResult.data, appType); - // Mark webhook event as processed if (logResult.webhookEventId) { try { await updateWebhookEvent(logResult.webhookEventId, { @@ -425,7 +400,6 @@ export async function handleGitHubWebhook( const result = await handleInstallationUnsuspend(parseResult.data, appType); - // Mark webhook event as processed if (logResult.webhookEventId) { try { await updateWebhookEvent(logResult.webhookEventId, { @@ -517,7 +491,6 @@ export async function handleGitHubWebhook( return result; } - // Handle installation_repositories events if (eventType === GITHUB_EVENT.INSTALLATION_REPOSITORIES) { const parseResult = InstallationRepositoriesPayloadSchema.safeParse(payload); if (!parseResult.success) { @@ -580,7 +553,6 @@ export async function handleGitHubWebhook( return result; } - // For other events, verify integration exists and is not suspended const installation = (payload as { installation?: { id?: number } }).installation; const installationId = installation?.id?.toString(); @@ -622,7 +594,6 @@ export async function handleGitHubWebhook( return NextResponse.json({ message: 'Event received' }, { status: 200 }); } - // Handle push events if (eventType === GITHUB_EVENT.PUSH) { const parseResult = PushEventPayloadSchema.safeParse(payload); if (!parseResult.success) { @@ -635,7 +606,6 @@ export async function handleGitHubWebhook( } if (!parseResult.data.deleted) { - // Process async after(async () => { if (!(await isAvailableForDeferredGitHubDispatch(integration))) return; await handlePushEvent(parseResult.data, integration); @@ -644,7 +614,6 @@ export async function handleGitHubWebhook( return NextResponse.json({ message: 'Event received' }, { status: 200 }); } - // Handle pull_request events if (eventType === GITHUB_EVENT.PULL_REQUEST) { const parseResult = PullRequestPayloadSchema.safeParse(payload); if (!parseResult.success) { @@ -741,7 +710,6 @@ export async function handleGitHubWebhook( return result; } - // Handle pull_request_review events — update cached review decision. if (eventType === GITHUB_EVENT.PULL_REQUEST_REVIEW) { const parseResult = PullRequestReviewPayloadSchema.safeParse(payload); if (!parseResult.success) { diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/index.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/index.ts index c838d7c58b..a4c6776c71 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/index.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/index.ts @@ -1,8 +1,3 @@ -/** - * GitHub Webhook Handlers - * Exports all webhook event handlers - */ - export { handleInstallationCreated, handleInstallationDeleted, diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/installation-repositories-handler.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/installation-repositories-handler.ts index d20545b7a5..0b5d5b1d7f 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/installation-repositories-handler.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/installation-repositories-handler.ts @@ -6,11 +6,6 @@ import { GITHUB_ACTION } from '@/lib/integrations/core/constants'; import { logExceptInTest } from '@/lib/utils.server'; import type { GitHubAppType } from '../app-selector'; -/** - * GitHub Installation Repositories Event Handler - * Handles: repositories added/removed - */ - export async function handleInstallationRepositories( payload: InstallationRepositoriesPayload, appType: GitHubAppType diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/issue-handler.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/issue-handler.ts index aba1ee9a53..9871b39ae6 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/issue-handler.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/issue-handler.ts @@ -7,17 +7,6 @@ import { IssueWebhookProcessor } from '@/lib/auto-triage/application/webhook/iss import { ConfigValidator } from '@/lib/auto-triage/application/webhook/config-validator'; import { LabelWebhookProcessor } from '@/lib/auto-fix/application/webhook/label-webhook-processor'; -/** - * GitHub Issue Event Handler for Auto Triage - * Handles: opened, reopened - * - * This handler processes GitHub issue events and creates triage tickets - * for automatic analysis, duplicate detection, and classification. - */ - -/** - * Issue payload type inferred from schema - */ type IssuePayload = { action: 'opened' | 'reopened' | 'edited'; issue: { @@ -42,10 +31,6 @@ type IssuePayload = { }; }; -/** - * Handles issue events that trigger auto triage - * (opened, reopened) - */ export async function handleIssueAutoTriage( payload: IssuePayload, integration: PlatformIntegration @@ -55,11 +40,7 @@ export async function handleIssueAutoTriage( return processor.process(payload, integration); } -/** - * Handles issue labeled events for Auto Fix - */ export async function handleIssueLabeled(payload: unknown, integration: PlatformIntegration) { - // Validate payload structure for labeled event const parseResult = IssueLabeledPayloadSchema.safeParse(payload); if (!parseResult.success) { @@ -74,25 +55,17 @@ export async function handleIssueLabeled(payload: unknown, integration: Platform return processor.process(parseResult.data, integration); } -/** - * Main router for issue events - * Routes to appropriate handler based on action - */ export async function handleIssue(payload: unknown, integration: PlatformIntegration) { - // Check action first to route to correct validator const action = (payload as { action?: string }).action; - // Handle labeled action separately (different schema) if (action === 'labeled') { return handleIssueLabeled(payload, integration); } - // Ignore unlabeled events - we only care about when labels are added if (action === 'unlabeled') { return NextResponse.json({ message: 'Event received' }, { status: 200 }); } - // Validate payload structure for other actions const parseResult = WebhookIssuePayloadSchema.safeParse(payload); if (!parseResult.success) { @@ -105,13 +78,11 @@ export async function handleIssue(payload: unknown, integration: PlatformIntegra const validatedPayload = parseResult.data; - // Handle opened and reopened actions switch (validatedPayload.action) { case 'opened': case 'reopened': return handleIssueAutoTriage(validatedPayload, integration); case 'edited': - // Ignore edited events for now // TODO: Add support for edited events return NextResponse.json({ message: 'Event received' }, { status: 200 }); default: diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts index 05257ee1f6..07577e32b7 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts @@ -504,14 +504,14 @@ describe('handlePullRequest', () => { it('routes a bot PR with a merge-commit head through skip, not merge-commit migration', async () => { mockGetBotUserId.mockResolvedValue('bot-user-1'); mockGetAgentConfigForOwner.mockResolvedValue({ is_enabled: true, config: {} }); - // A non-bot PR here would hit the merge-commit path (step 4) and return 'Skipped merge commit'. + // A non-bot PR here would hit the merge-commit path and return 'Skipped merge commit'. mockIsMergeCommit.mockResolvedValue(true); const payload = pullRequestPayload(); payload.pull_request.user.type = 'Bot'; const response = await handlePullRequest(payload, platformIntegration()); - // The bot-skip decision defers step 4, so the PR is skipped as a bot PR (not re-pointed to a + // The bot-skip decision defers the merge-commit path, so the PR is skipped as a bot PR (not re-pointed to a // new SHA with a fresh check run by migrateInFlightReviewsToMergeCommitHead). expect(response.status).toBe(200); expect(await response.json()).toEqual({ message: 'Skipped bot-authored PR' }); diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts index 79b964c541..68a1b3fe15 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts @@ -38,10 +38,6 @@ import { resolvePullRequestCheckoutRef } from './pull-request-checkout-ref'; import { APP_URL } from '@/lib/constants'; import { getCodeReviewActionRequiredState } from '@/lib/code-reviews/action-required'; -/** - * GitHub Pull Request Event Handler - * Handles: opened, synchronize, reopened - /** * Handles pull request events that trigger code review * (opened, synchronize, reopened) @@ -71,7 +67,6 @@ export async function handlePullRequestCodeReview( checkoutRef: checkoutRef.checkoutRef, }); - // Skip draft PRs - only trigger code review for ready PRs if (pull_request.draft === true) { logExceptInTest('Skipping draft PR:', { pr_number: pull_request.number, @@ -80,7 +75,6 @@ export async function handlePullRequestCodeReview( return NextResponse.json({ message: 'Skipped draft PR' }, { status: 200 }); } - // Debug: Log integration fields logExceptInTest('Integration fields:', { id: integration.id, owned_by_organization_id: integration.owned_by_organization_id, @@ -88,8 +82,6 @@ export async function handlePullRequestCodeReview( kilo_requester_user_id: integration.kilo_requester_user_id, }); - // 1. Determine owner from integration - // For orgs: use bot user, fallback to integration creator const orgBotUserId = integration.owned_by_organization_id ? await getBotUserId(integration.owned_by_organization_id, 'code-review') : null; @@ -98,7 +90,6 @@ export async function handlePullRequestCodeReview( ? { type: 'org', id: integration.owned_by_organization_id, - // Use bot user if available, fallback to integration creator userId: (orgBotUserId ?? integration.kilo_requester_user_id) as string, } : { @@ -107,7 +98,6 @@ export async function handlePullRequestCodeReview( userId: integration.owned_by_user_id as string, }; - // Validate we have a valid user ID if (!owner.userId) { logExceptInTest('No valid user ID found for integration:', { integrationId: integration.id, @@ -121,7 +111,6 @@ export async function handlePullRequestCodeReview( ); } - // 2. Check if code review agent is enabled for this owner const agentConfig = await getAgentConfigForOwner(owner, 'code_review', 'github'); if (!agentConfig || !agentConfig.is_enabled || getCodeReviewActionRequiredState(agentConfig)) { @@ -138,11 +127,10 @@ export async function handlePullRequestCodeReview( `Code review agent enabled for ${owner.type} ${owner.id}, processing ${repository.full_name}#${pull_request.number}` ); - // 3. Check if repository is in allowed list (when using selected repositories mode) const config = agentConfig.config as CodeReviewAgentConfig; - // Bot PRs are skipped by default (enforced at step 5b). Compute the decision up front so the - // merge-commit path (step 4) also defers to it: otherwise a bot PR whose head is a merge commit + // Bot PRs are skipped by default (enforced below). Compute the decision up front so the + // merge-commit path also defers to it: otherwise a bot PR whose head is a merge commit // would be re-pointed to a new SHA with a fresh check run, keeping alive a review the guardrail // is meant to skip. When true, the PR falls through to cancellation + skip instead. const skipBotPullRequests = config.skip_bot_pull_requests ?? true; @@ -180,7 +168,7 @@ export async function handlePullRequestCodeReview( const headFullName = checkoutRef.headRepoFullName ?? repository.full_name; const [headOwner, headRepoName] = headFullName.split('/'); - // 4. Skip merge commits on synchronize (e.g. merging base branch into feature branch). + // Skip merge commits on synchronize (e.g. merging base branch into feature branch). // Runs before cancellation so that an in-flight review at an earlier SHA is preserved: // a merge commit introduces no new feature work and should not supersede the existing review. if ( @@ -223,7 +211,7 @@ export async function handlePullRequestCodeReview( return NextResponse.json({ message: 'Skipped merge commit' }, { status: 200 }); } - // 5. Cancel any existing reviews for this PR (different SHA) + // Cancel any existing reviews for this PR (different SHA). // This prevents spam when user pushes multiple commits quickly const cancelledReviews = await cancelSupersededReviewsForPR(reviewScope, pull_request.head.sha); @@ -297,12 +285,12 @@ export async function handlePullRequestCodeReview( ); } - // 5b. Feature-level guardrail: by default, skip automated reviews of bot-authored PRs + // Feature-level guardrail: by default, skip automated reviews of bot-authored PRs // (dependabot/renovate/etc.) — high-volume, low-value dependency bumps otherwise consume review - // compute and clutter the PR. Configurable per org via `skip_bot_pull_requests` (see the - // decision computed before step 4). Applies to standard and council reviews; manual reviews - // never reach this handler. Runs AFTER supersession (step 5) so a bot push still cancels any - // stale in-flight review and resolves its check run, instead of leaving it stuck. + // compute and clutter the PR. Configurable per org via `skip_bot_pull_requests`. Applies to + // standard and council reviews; manual reviews never reach this handler. Runs AFTER + // supersession so a bot push still cancels any stale in-flight review and resolves its + // check run, instead of leaving it stuck. if (isBotPullRequestSkip) { logExceptInTest('Skipping bot-authored PR:', { pr_number: pull_request.number, @@ -312,7 +300,6 @@ export async function handlePullRequestCodeReview( return NextResponse.json({ message: 'Skipped bot-authored PR' }, { status: 200 }); } - // 6. Check for duplicate review (same repo, PR, SHA) const existingReview = await findExistingReview(reviewScope, pull_request.head.sha); if (existingReview) { @@ -329,7 +316,7 @@ export async function handlePullRequestCodeReview( ); } - // 6b. Decide standard vs council for this automated review. Council is a per-repo opt-in and + // Decide standard vs council for this automated review. Council is a per-repo opt-in and // requires an active council config + entitlement. The entitlement lookup is a DB call, so it // is gated behind the two cheap local checks (most webhooks are standard and skip it). Falls // back to 'standard' if any condition is missing, so a bad/absent council config never blocks. @@ -371,7 +358,7 @@ export async function handlePullRequestCodeReview( ); } - // 7. Create review record (session_id will be updated async) + // The review record's session_id is updated async. const reviewId = await createCodeReview({ owner, reviewType, @@ -395,7 +382,6 @@ export async function handlePullRequestCodeReview( const [repoOwner, repoName] = repository.full_name.split('/'); - // 8. Create GitHub Check Run (PR gate) — skip for lite (read-only) app if (appType !== 'lite') { let checkRunId: number | undefined; try { @@ -444,7 +430,6 @@ export async function handlePullRequestCodeReview( } } - // 9. Post 👀 reaction to show Kilo is reviewing try { await addReactionToPR( integration.platform_installation_id as string, @@ -459,7 +444,6 @@ export async function handlePullRequestCodeReview( logExceptInTest('Failed to add eyes reaction:', reactionError); } - // 10. Try to dispatch pending reviews (including this new one) // Review is created with status='pending' and dispatch will pick it up if slots available try { const dispatchResult = await tryDispatchPendingReviews(owner); @@ -484,7 +468,6 @@ export async function handlePullRequestCodeReview( // Don't throw - review record created as pending, will be picked up later } - // 11. Return 202 Accepted (always succeeds, review queued as pending) return NextResponse.json( { message: 'Code review queued', @@ -619,9 +602,6 @@ async function migrateInFlightReviewsToMergeCommitHead(args: { } } -/** - * Main router for pull request events - */ export async function handlePullRequest( payload: PullRequestPayload, integration: PlatformIntegration diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/upsert-cli-session-pull-requests.test.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/upsert-cli-session-pull-requests.test.ts index 9a0601b4aa..9c021f2a38 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/upsert-cli-session-pull-requests.test.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/upsert-cli-session-pull-requests.test.ts @@ -648,7 +648,6 @@ describe('upsertCliSessionPullRequestsFromWebhook', () => { const branch = 'feature/missed-reopen-then-sync'; await seedSession({ branch, owner: testOwner }); - // PR gets closed. await upsertCliSessionPullRequestsFromWebhook( makePayload({ action: 'closed', diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-helpers.ts b/apps/web/src/lib/integrations/platforms/github/webhook-helpers.ts index 09f88667c8..254f388b20 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-helpers.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-helpers.ts @@ -4,13 +4,6 @@ import type { PendingApprovalMetadata, } from '@/lib/integrations/core/types'; -/** - * Helper functions for GitHub webhook processing - */ - -/** - * Extract pending approval metadata from a platform integration record - */ export function extractPendingApprovalMetadata( integration: PlatformIntegration ): PendingApprovalMetadata | null { @@ -19,9 +12,6 @@ export function extractPendingApprovalMetadata( return pendingApproval || null; } -/** - * Build installation data object from GitHub webhook payload - */ export function buildInstallationData(installation: { id: number; account: { diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-schemas.ts b/apps/web/src/lib/integrations/platforms/github/webhook-schemas.ts index 5d1c8e2881..b5269ed166 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-schemas.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-schemas.ts @@ -1,11 +1,5 @@ import * as z from 'zod'; -/** - * Zod schemas for GitHub webhook payload validation - * These ensure we receive the expected data structure from GitHub - */ - -// Common schemas used across multiple webhook types const GitHubAccountSchema = z.object({ id: z.number(), login: z.string(), @@ -38,7 +32,6 @@ export const GitHubAppAuthorizationRevokedPayloadSchema = z.object({ }), }); -// installation.created webhook payload export const InstallationCreatedPayloadSchema = z.object({ action: z.literal('created'), installation: GitHubInstallationSchema, @@ -46,7 +39,6 @@ export const InstallationCreatedPayloadSchema = z.object({ sender: GitHubSenderSchema.optional(), }); -// installation.deleted webhook payload export const InstallationDeletedPayloadSchema = z.object({ action: z.literal('deleted'), installation: z.object({ @@ -59,7 +51,6 @@ export const InstallationDeletedWebhookPayloadSchema = InstallationDeletedPayloa installation: InstallationDeletedPayloadSchema.shape.installation.nullable(), }); -// installation.suspend webhook payload export const InstallationSuspendPayloadSchema = z.object({ action: z.literal('suspend'), installation: z.object({ @@ -68,7 +59,6 @@ export const InstallationSuspendPayloadSchema = z.object({ sender: GitHubSenderSchema.optional(), }); -// installation.unsuspend webhook payload export const InstallationUnsuspendPayloadSchema = z.object({ action: z.literal('unsuspend'), installation: z.object({ @@ -77,7 +67,6 @@ export const InstallationUnsuspendPayloadSchema = z.object({ sender: GitHubSenderSchema.optional(), }); -// installation_target.renamed webhook payload export const InstallationTargetRenamedPayloadSchema = z.object({ action: z.literal('renamed'), installation: z.object({ @@ -88,7 +77,6 @@ export const InstallationTargetRenamedPayloadSchema = z.object({ target_type: z.string(), }); -// installation_repositories webhook payload export const InstallationRepositoriesPayloadSchema = z.object({ action: z.enum(['added', 'removed']), installation: z.object({ @@ -116,7 +104,6 @@ export const InstallationRepositoriesPayloadSchema = z.object({ .optional(), }); -// push webhook payload export const PushEventPayloadSchema = z.object({ ref: z.string(), repository: z.object({ @@ -125,7 +112,6 @@ export const PushEventPayloadSchema = z.object({ deleted: z.boolean(), }); -// pull_request webhook payload export const GitHubRepositorySchema = z.object({ id: z.number(), name: z.string(), @@ -185,7 +171,6 @@ export const PullRequestPayloadSchema = z.object({ sender: GitHubSenderSchema.optional(), }); -// issues webhook payload export const IssuePayloadSchema = z.object({ action: z.string(), issue: z.object({ @@ -237,7 +222,6 @@ export const GitHubAuthorAssociationSchema = z.enum([ 'OWNER', ]); -// pull_request_review_comment webhook payload export const PullRequestReviewCommentPayloadSchema = z.object({ action: z.string(), comment: z.object({ @@ -277,7 +261,6 @@ export const PullRequestReviewCommentPayloadSchema = z.object({ sender: GitHubSenderSchema.optional(), }); -// pull_request_review webhook payload export const PullRequestReviewPayloadSchema = z.object({ action: z.enum(['submitted', 'edited', 'dismissed']), review: z.object({ @@ -312,7 +295,6 @@ export const PullRequestReviewPayloadSchema = z.object({ installation: z.object({ id: z.number() }), }); -// Type exports for use in the webhook handler export type GitHubAppAuthorizationRevokedPayload = z.infer< typeof GitHubAppAuthorizationRevokedPayloadSchema >; diff --git a/apps/web/src/lib/integrations/platforms/gitlab/adapter.ts b/apps/web/src/lib/integrations/platforms/gitlab/adapter.ts index cae231422d..71211d0cc3 100644 --- a/apps/web/src/lib/integrations/platforms/gitlab/adapter.ts +++ b/apps/web/src/lib/integrations/platforms/gitlab/adapter.ts @@ -1,10 +1,3 @@ -/** - * GitLab API Adapter - * - * Provides OAuth-based authentication and API operations for GitLab. - * Supports both GitLab.com and self-hosted GitLab instances. - */ - import { getEnvVariable } from '@/lib/dotenvx'; import { PLATFORM } from '@/lib/integrations/core/constants'; import type { PlatformRepository } from '@/lib/integrations/core/types'; @@ -252,9 +245,6 @@ function responseHeadersToHeaders(headers: http.IncomingHttpHeaders): Headers { return responseHeaders; } -/** - * GitLab OAuth scopes required for the integration - */ export const GITLAB_OAUTH_SCOPES = [ 'api', // Full API access (needed for MR comments, reactions) 'read_user', // Read user info @@ -262,9 +252,6 @@ export const GITLAB_OAUTH_SCOPES = [ 'write_repository', // Push branches (for auto-fix) ] as const; -/** - * GitLab API response types - */ export type GitLabUser = { id: number; username: string; @@ -305,21 +292,11 @@ export type GitLabOAuthTokens = { scope: string; }; -/** - * OAuth credentials type for self-hosted GitLab instances - */ export type GitLabOAuthCredentials = { clientId: string; clientSecret: string; }; -/** - * Builds the GitLab OAuth authorization URL - * - * @param state - State parameter for CSRF protection (e.g., "org_xxx" or "user_xxx") - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - * @param customCredentials - Optional custom OAuth credentials for self-hosted instances - */ export function buildGitLabOAuthUrl( state: string, instanceUrl: string = DEFAULT_GITLAB_URL, @@ -347,13 +324,6 @@ export function buildGitLabOAuthUrl( return buildGitLabUrl(normalizedInstanceUrl, '/oauth/authorize', Object.fromEntries(params)); } -/** - * Exchanges an OAuth authorization code for access and refresh tokens - * - * @param code - The authorization code from the OAuth callback - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - * @param customCredentials - Optional custom OAuth credentials for self-hosted instances - */ export async function exchangeGitLabOAuthCode( code: string, instanceUrl: string = DEFAULT_GITLAB_URL, @@ -403,12 +373,6 @@ export async function exchangeGitLabOAuthCode( return tokens; } -/** - * Fetches the authenticated GitLab user's information - * - * @param accessToken - OAuth access token - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - */ export async function fetchGitLabUser( accessToken: string, instanceUrl: string = DEFAULT_GITLAB_URL @@ -428,12 +392,6 @@ export async function fetchGitLabUser( return (await response.json()) as GitLabUser; } -/** - * Fetches all projects (repositories) accessible by the authenticated user - * - * @param accessToken - OAuth access token - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - */ export async function fetchGitLabProjects( accessToken: string, instanceUrl: string = DEFAULT_GITLAB_URL @@ -489,13 +447,6 @@ export async function fetchGitLabProjects( return projects; } -/** - * Fetches all branches for a GitLab project - * - * @param accessToken - OAuth access token - * @param projectId - GitLab project ID or path (URL-encoded) - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - */ export async function fetchGitLabBranches( accessToken: string, projectId: string | number, @@ -529,7 +480,6 @@ export async function fetchGitLabBranches( const data = (await response.json()) as GitLabBranch[]; branches.push(...data); - // Check if there are more pages const totalPages = parseInt(response.headers.get('x-total-pages') || '1', 10); if (page >= totalPages) break; page++; @@ -583,27 +533,13 @@ export async function fetchGitLabRootTextFileAtRef( return await response.text(); } -/** - * Calculates the expiration timestamp from GitLab OAuth response - * - * @param createdAt - Unix timestamp when token was created - * @param expiresIn - Seconds until expiration - */ export function calculateTokenExpiry(createdAt: number, expiresIn: number): string { const expiresAtMs = (createdAt + expiresIn) * 1000; return new Date(expiresAtMs).toISOString(); } -// ============================================================================ -// Webhook Verification -// ============================================================================ - /** - * Verifies GitLab webhook token - * GitLab uses a simple secret token comparison (not HMAC like GitHub) - * - * @param token - The token from X-Gitlab-Token header - * @param expectedToken - The expected webhook secret (optional, uses env var if not provided) + * GitLab uses a simple secret token comparison (not HMAC like GitHub). */ export function verifyGitLabWebhookToken(token: string, expectedToken?: string): boolean { if (!expectedToken) { @@ -619,10 +555,6 @@ export function verifyGitLabWebhookToken(token: string, expectedToken?: string): } } -// ============================================================================ -// Webhook Management API Functions -// ============================================================================ - /** * Custom error class for webhook permission issues * Thrown when user doesn't have Maintainer+ role on a project @@ -638,9 +570,6 @@ export class GitLabWebhookPermissionError extends Error { } } -/** - * GitLab Project Webhook type - */ export type GitLabWebhook = { id: number; url: string; @@ -697,7 +626,6 @@ export async function listProjectWebhooks( projectId, }); - // 401/403 indicate permission issues - user doesn't have Maintainer+ role if (response.status === 401 || response.status === 403) { throw new GitLabWebhookPermissionError( projectId, @@ -770,7 +698,6 @@ export async function createProjectWebhook( projectId, }); - // 401/403 indicate permission issues - user doesn't have Maintainer+ role if (response.status === 401 || response.status === 403) { throw new GitLabWebhookPermissionError( projectId, @@ -854,7 +781,6 @@ export async function updateProjectWebhook( hookId, }); - // 401/403 indicate permission issues - user doesn't have Maintainer+ role if (response.status === 401 || response.status === 403) { throw new GitLabWebhookPermissionError( projectId, @@ -923,31 +849,16 @@ export async function deleteProjectWebhook( }); } -/** - * Normalizes a URL for comparison by decoding percent-encoded characters - * and ensuring consistent formatting - */ function normalizeUrlForComparison(url: string): string { try { - // Decode the URL to handle percent-encoded characters const decoded = decodeURIComponent(url); - // Parse and re-stringify to normalize the URL format const parsed = new URL(decoded); return parsed.toString(); } catch { - // If URL parsing fails, return the original URL return url; } } -/** - * Finds an existing Kilo webhook on a GitLab project by URL - * - * @param accessToken - OAuth access token - * @param projectId - GitLab project ID or path (URL-encoded) - * @param kiloWebhookUrl - The Kilo webhook URL to search for - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - */ export async function findKiloWebhook( accessToken: string, projectId: string | number, @@ -956,10 +867,8 @@ export async function findKiloWebhook( ): Promise { const webhooks = await listProjectWebhooks(accessToken, projectId, instanceUrl); - // Normalize the target URL for comparison const normalizedTargetUrl = normalizeUrlForComparison(kiloWebhookUrl); - // Find webhook by comparing normalized URLs const kiloWebhook = webhooks.find( hook => normalizeUrlForComparison(hook.url) === normalizedTargetUrl ); @@ -979,10 +888,6 @@ export async function findKiloWebhook( return kiloWebhook || null; } -// ============================================================================ -// Commit Inspection -// ============================================================================ - /** * Checks whether a commit is a merge commit (has 2+ parent IDs). * Used to skip code reviews triggered by "merge base into feature" pushes. @@ -1039,13 +944,6 @@ export async function isMergeCommit( } } -// ============================================================================ -// Merge Request API Functions -// ============================================================================ - -/** - * GitLab MR Note (comment) type - */ export type GitLabNote = { id: number; body: string; @@ -1079,18 +977,12 @@ export type GitLabNote = { }; }; -/** - * GitLab MR Discussion type (threaded comments) - */ export type GitLabDiscussion = { id: string; individual_note: boolean; notes: GitLabNote[]; }; -/** - * GitLab Merge Request type - */ export type GitLabMergeRequest = { id: number; iid: number; @@ -1147,13 +1039,7 @@ export async function fetchGitLabMergeRequest(params: { } /** - * Finds an existing Kilo review note on a GitLab MR - * Looks for the marker in MR notes - * - * @param accessToken - OAuth access token - * @param projectId - GitLab project ID or path (URL-encoded) - * @param mrIid - Merge request internal ID - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) + * Looks for the marker in MR notes. */ export async function findKiloReviewNote( accessToken: string, @@ -1202,11 +1088,9 @@ export async function findKiloReviewNote( totalNotes: notes.length, }); - // Look for notes with the kilo-review marker const markedNotes = notes.filter(n => n.body?.includes('') && !n.system); if (markedNotes.length > 0) { - // Sort by updated_at descending and pick the latest const latestNote = markedNotes.sort((a, b) => { return new Date(b.updated_at).getTime() - new Date(a.updated_at).getTime(); })[0]; @@ -1230,10 +1114,6 @@ export async function findKiloReviewNote( return null; } -/** - * Updates an existing Kilo review note on a GitLab MR - * Used to append usage footer (model + token count) after review completion - */ export async function updateKiloReviewNote( accessToken: string, projectId: string | number, @@ -1272,9 +1152,6 @@ export async function updateKiloReviewNote( }); } -/** - * Creates a new top-level note on a GitLab MR. - */ export async function createMRNote( accessToken: string, projectId: string | number, @@ -1358,13 +1235,7 @@ export async function hasMRNoteWithMarker( } /** - * Fetches existing inline comments (discussions) on a GitLab MR - * Used to detect duplicates and track outdated comments - * - * @param accessToken - OAuth access token - * @param projectId - GitLab project ID or path (URL-encoded) - * @param mrIid - Merge request internal ID - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) + * Used to detect duplicates and track outdated inline comments. */ export async function fetchMRInlineComments( accessToken: string, @@ -1417,7 +1288,6 @@ export async function fetchMRInlineComments( page++; } - // Extract inline comments from discussions const inlineComments: Array<{ id: number; discussionId: string; @@ -1429,11 +1299,9 @@ export async function fetchMRInlineComments( }> = []; for (const discussion of discussions) { - // Skip individual notes (non-threaded comments) if (discussion.individual_note) continue; for (const note of discussion.notes) { - // Only include notes with position (inline comments) if (note.position) { inlineComments.push({ id: note.id, @@ -1459,14 +1327,6 @@ export async function fetchMRInlineComments( return inlineComments; } -/** - * Gets the HEAD commit SHA for a GitLab MR - * - * @param accessToken - OAuth access token - * @param projectId - GitLab project ID or path (URL-encoded) - * @param mrIid - Merge request internal ID - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - */ export async function getMRHeadCommit( accessToken: string, projectId: string | number, @@ -1503,13 +1363,7 @@ export async function getMRHeadCommit( } /** - * Gets the diff refs (base, head, start SHA) for a GitLab MR - * Required for creating inline comments - * - * @param accessToken - OAuth access token - * @param projectId - GitLab project ID or path (URL-encoded) - * @param mrIid - Merge request internal ID - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) + * The diff refs are required for creating inline comments. */ export async function getMRDiffRefs( accessToken: string, @@ -1553,14 +1407,7 @@ export async function getMRDiffRefs( } /** - * Adds an award emoji (reaction) to a GitLab MR - * Used to show that Kilo is reviewing an MR (e.g., 👀 eyes reaction) - * - * @param accessToken - OAuth access token - * @param projectId - GitLab project ID or path (URL-encoded) - * @param mrIid - Merge request internal ID - * @param emoji - Emoji name (e.g., 'eyes', 'thumbsup', 'thumbsdown') - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) + * Used to show that Kilo is reviewing an MR (e.g., 👀 eyes reaction). */ export async function addReactionToMR( accessToken: string, @@ -1610,13 +1457,6 @@ export async function addReactionToMR( }); } -/** - * Gets a GitLab project by path - * - * @param accessToken - OAuth access token - * @param projectPath - Project path (e.g., "group/project") - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - */ export async function getGitLabProject( accessToken: string, projectPath: string, @@ -1689,13 +1529,6 @@ function readGitLabRepositorySizeBytes(data: unknown): number | null { return typeof repositorySize === 'number' ? repositorySize : null; } -// ============================================================================ -// Instance Validation -// ============================================================================ - -/** - * GitLab version response type - */ export type GitLabVersion = { version: string; revision: string; @@ -1707,9 +1540,6 @@ export type GitLabVersion = { enterprise: boolean; }; -/** - * Result of validating a GitLab instance - */ export type GitLabInstanceValidationResult = { valid: boolean; version?: string; @@ -1719,13 +1549,8 @@ export type GitLabInstanceValidationResult = { }; /** - * Validates that a URL points to a valid GitLab instance - * * Uses the public /api/v4/version endpoint which doesn't require authentication. * This allows users to verify their self-hosted GitLab URL before attempting OAuth. - * - * @param instanceUrl - The GitLab instance URL to validate - * @returns Validation result with version info if successful */ export async function validateGitLabInstance( instanceUrl: string @@ -1741,14 +1566,12 @@ export async function validateGitLabInstance( } try { - // The /api/v4/version endpoint is public and doesn't require authentication const response = await fetchGitLab(buildGitLabUrl(normalizedUrl, '/api/v4/version'), { method: 'GET', headers: { Accept: 'application/json', }, redirect: 'manual', - // Set a reasonable timeout for the request signal: AbortSignal.timeout(10000), }); @@ -1780,7 +1603,6 @@ export async function validateGitLabInstance( const data = (await response.json()) as GitLabVersion; - // Validate that the response looks like a GitLab version response if (!data.version || typeof data.version !== 'string') { return { valid: false, @@ -1808,7 +1630,6 @@ export async function validateGitLabInstance( }; } - // Handle timeout if (error instanceof Error && error.name === 'TimeoutError') { return { valid: false, @@ -1816,7 +1637,6 @@ export async function validateGitLabInstance( }; } - // Handle network errors if (error instanceof TypeError && error.message.includes('fetch')) { return { valid: false, @@ -1836,10 +1656,6 @@ export async function validateGitLabInstance( } } -// ============================================================================ -// Project Access Token (PrAT) Management -// ============================================================================ - /** * GitLab access level constants * @see https://docs.gitlab.com/ee/api/members.html#valid-access-levels @@ -1947,7 +1763,6 @@ export async function createProjectAccessToken( projectId, }); - // 401/403 indicate permission issues - user doesn't have Maintainer+ role if (response.status === 401 || response.status === 403) { throw new GitLabProjectAccessTokenPermissionError( projectId, @@ -2238,7 +2053,6 @@ export async function findKiloProjectAccessToken( ): Promise { const tokens = await listProjectAccessTokens(accessToken, projectId, instanceUrl); - // Find active token with matching name const kiloToken = tokens.find(t => t.name === tokenName && t.active && !t.revoked); if (kiloToken) { @@ -2258,11 +2072,7 @@ export async function findKiloProjectAccessToken( } /** - * Calculates the expiration date for a new Project Access Token - * GitLab allows max 1 year expiration - * - * @param daysFromNow - Number of days from now (default: 365, max: 365) - * @returns Date string in YYYY-MM-DD format + * GitLab allows max 1 year expiration. */ export function calculateProjectAccessTokenExpiry(daysFromNow: number = 365): string { const maxDays = 365; @@ -2274,13 +2084,6 @@ export function calculateProjectAccessTokenExpiry(daysFromNow: number = 365): st return expiryDate.toISOString().split('T')[0]; } -/** - * Checks if a Project Access Token is expiring soon (within specified days) - * - * @param expiresAt - Expiration date in YYYY-MM-DD format - * @param withinDays - Number of days to consider "soon" (default: 7) - * @returns true if token expires within the specified days - */ export function isProjectAccessTokenExpiringSoon( expiresAt: string, withinDays: number = 7 @@ -2295,22 +2098,14 @@ export function isProjectAccessTokenExpiringSoon( } /** - * Validates a Project Access Token by making a test API call - * - * This is useful to check if a stored token is still valid on GitLab - * (e.g., it might have been manually revoked by the user) - * - * @param token - The Project Access Token to validate - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - * @returns true if the token is valid, false otherwise + * Checks whether a stored token is still valid on GitLab + * (e.g., it might have been manually revoked by the user). */ export async function validateProjectAccessToken( token: string, instanceUrl: string = DEFAULT_GITLAB_URL ): Promise { try { - // Use the /user endpoint to validate the token - // This is a lightweight call that works with any valid token const response = await fetchGitLab(buildGitLabUrl(instanceUrl, '/api/v4/user'), { headers: { Authorization: `Bearer ${token}`, @@ -2348,10 +2143,6 @@ export async function validateProjectAccessToken( } } -// ============================================================================ -// Personal Access Token (PAT) Validation -// ============================================================================ - /** * Required scopes for PAT-based authentication * - api: Full API access (needed for webhooks, MR comments, PrAT creation) @@ -2380,9 +2171,6 @@ export type GitLabPersonalAccessTokenInfo = { expires_at: string | null; }; -/** - * Result of validating a Personal Access Token - */ export type GitLabPATValidationResult = { valid: boolean; user?: GitLabUser; @@ -2399,19 +2187,6 @@ export type GitLabPATValidationResult = { warnings?: string[]; }; -/** - * Validates a GitLab Personal Access Token - * - * This function: - * 1. Calls /api/v4/personal_access_tokens/self to get token info (requires GitLab 14.0+) - * 2. Validates that 'api' scope is present - * 3. Fetches user info from /api/v4/user - * 4. Checks for expiration and adds warnings if expiring soon - * - * @param token - The Personal Access Token to validate - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - * @returns Validation result with user info, scopes, and any warnings - */ export async function validatePersonalAccessToken( token: string, instanceUrl: string = DEFAULT_GITLAB_URL @@ -2430,8 +2205,7 @@ export async function validatePersonalAccessToken( }; } - // Step 1: Get token info from /api/v4/personal_access_tokens/self - // This endpoint requires GitLab 14.0+ + // /api/v4/personal_access_tokens/self requires GitLab 14.0+ let tokenInfoResponse: Response; try { tokenInfoResponse = await fetchGitLab(tokenInfoUrl, { @@ -2481,7 +2255,6 @@ export async function validatePersonalAccessToken( const tokenInfo = (await tokenInfoResponse.json()) as GitLabPersonalAccessTokenInfo; - // Check if token is revoked or inactive if (tokenInfo.revoked || !tokenInfo.active) { logExceptInTest('[validatePersonalAccessToken] Token is revoked or inactive', { revoked: tokenInfo.revoked, @@ -2493,7 +2266,6 @@ export async function validatePersonalAccessToken( }; } - // Step 2: Validate required scopes const missingScopes = GITLAB_PAT_REQUIRED_SCOPES.filter( scope => !tokenInfo.scopes.includes(scope) ); @@ -2511,7 +2283,6 @@ export async function validatePersonalAccessToken( }; } - // Step 3: Check expiration and add warnings if (tokenInfo.expires_at) { const expiresAt = new Date(tokenInfo.expires_at); const now = new Date(); @@ -2536,7 +2307,6 @@ export async function validatePersonalAccessToken( } } - // Step 4: Fetch user info let userResponse: Response; try { userResponse = await fetchGitLab(userUrl, { @@ -2594,10 +2364,6 @@ export async function validatePersonalAccessToken( }; } -// ============================================================================ -// Commit Status API (MR gate checks) -// ============================================================================ - /** * GitLab commit status states. * @see https://docs.gitlab.com/ee/api/commits.html#set-the-pipeline-status-of-a-commit @@ -2605,21 +2371,12 @@ export async function validatePersonalAccessToken( export type GitLabCommitStatusState = 'pending' | 'running' | 'success' | 'failed' | 'canceled'; /** - * Sets (creates or updates) a commit status on a GitLab commit. - * * GitLab commit statuses are idempotent by (sha, name): posting the same * name+sha combination updates the existing status rather than creating * a duplicate. This means we don't need to track a status ID like GitHub. * * The status appears in the MR pipeline widget and can be configured as * a required external approval in merge request approval rules. - * - * @param accessToken - OAuth or Project Access Token - * @param projectId - GitLab project ID or path - * @param sha - The commit SHA to attach the status to - * @param state - Status state - * @param options - Additional options (targetUrl, description) - * @param instanceUrl - GitLab instance URL */ export async function setCommitStatus( accessToken: string, diff --git a/apps/web/src/lib/integrations/platforms/gitlab/webhook-handlers/index.ts b/apps/web/src/lib/integrations/platforms/gitlab/webhook-handlers/index.ts index d7b8689e15..d8e2fe741b 100644 --- a/apps/web/src/lib/integrations/platforms/gitlab/webhook-handlers/index.ts +++ b/apps/web/src/lib/integrations/platforms/gitlab/webhook-handlers/index.ts @@ -1,7 +1 @@ -/** - * GitLab Webhook Handlers - Barrel Export - * - * Re-exports all GitLab webhook handlers for convenient importing. - */ - export { handleMergeRequest, handleMergeRequestCodeReview } from './merge-request-handler'; diff --git a/apps/web/src/lib/integrations/platforms/gitlab/webhook-handlers/merge-request-handler.ts b/apps/web/src/lib/integrations/platforms/gitlab/webhook-handlers/merge-request-handler.ts index 00ad4e59dd..bc9c21359a 100644 --- a/apps/web/src/lib/integrations/platforms/gitlab/webhook-handlers/merge-request-handler.ts +++ b/apps/web/src/lib/integrations/platforms/gitlab/webhook-handlers/merge-request-handler.ts @@ -1,12 +1,3 @@ -/** - * GitLab Merge Request Event Handler - * - * Handles merge request events that trigger code review: - * - open: New MR created - * - update: MR updated (new commits pushed) - * - reopen: MR reopened - */ - import { NextResponse } from 'next/server'; import { addBreadcrumb, captureException } from '@sentry/nextjs'; import type { MergeRequestPayload } from '../webhook-schemas'; @@ -43,10 +34,6 @@ import type { GitLabCredentialActor } from '../credential-broker-client'; const SUPERSEDED_BY_NEW_PUSH_REASON = 'Superseded by new push'; const DUPLICATE_MERGE_CONTINUATION_REASON = 'Superseded by duplicate merge-commit continuation'; -/** - * Handles merge request events that trigger code review - * (open, update, reopen) - */ export async function handleMergeRequestCodeReview( payload: MergeRequestPayload, integration: PlatformIntegration @@ -62,7 +49,6 @@ export async function handleMergeRequestCodeReview( author: payload.user?.username, }); - // Skip draft/WIP MRs - only trigger code review for ready MRs if (mr.draft === true || mr.work_in_progress === true) { logExceptInTest('Skipping draft/WIP MR:', { mr_iid: mr.iid, @@ -71,7 +57,6 @@ export async function handleMergeRequestCodeReview( return NextResponse.json({ message: 'Skipped draft MR' }, { status: 200 }); } - // Debug: Log integration fields logExceptInTest('Integration fields:', { id: integration.id, owned_by_organization_id: integration.owned_by_organization_id, @@ -79,8 +64,6 @@ export async function handleMergeRequestCodeReview( kilo_requester_user_id: integration.kilo_requester_user_id, }); - // 1. Determine owner from integration - // For orgs: use bot user, fallback to integration creator const orgBotUserId = integration.owned_by_organization_id ? await getBotUserId(integration.owned_by_organization_id, 'code-review') : null; @@ -89,7 +72,6 @@ export async function handleMergeRequestCodeReview( ? { type: 'org', id: integration.owned_by_organization_id, - // Use bot user if available, fallback to integration creator userId: (orgBotUserId ?? integration.kilo_requester_user_id) as string, } : { @@ -98,7 +80,6 @@ export async function handleMergeRequestCodeReview( userId: integration.owned_by_user_id as string, }; - // Validate we have a valid user ID if (!owner.userId) { logExceptInTest('No valid user ID found for integration:', { integrationId: integration.id, @@ -113,7 +94,6 @@ export async function handleMergeRequestCodeReview( ...(owner.type === 'org' ? { organizationId: owner.id } : {}), }; - // 2. Check if code review agent is enabled for this owner (GitLab platform) const agentConfig = await getAgentConfigForOwner(owner, 'code_review', PLATFORM.GITLAB); if (!agentConfig || !agentConfig.is_enabled) { @@ -130,13 +110,11 @@ export async function handleMergeRequestCodeReview( `Code review agent enabled for ${owner.type} ${owner.id}, processing ${project.path_with_namespace}!${mr.iid}` ); - // 3. Check if repository is in allowed list (when using selected repositories mode) const config = agentConfig.config as CodeReviewAgentConfig; if ( config?.repository_selection_mode === 'selected' && Array.isArray(config?.selected_repository_ids) ) { - // Check both selected_repository_ids and manually_added_repositories const isInSelectedList = config.selected_repository_ids.includes(project.id); const isInManuallyAddedList = Array.isArray(config.manually_added_repositories) ? config.manually_added_repositories.some(repo => repo.id === project.id) @@ -158,7 +136,6 @@ export async function handleMergeRequestCodeReview( ); } - // Get the head SHA from the last commit const headSha = mr.last_commit?.id; if (!headSha) { logExceptInTest('No head commit SHA found in MR payload:', { @@ -176,7 +153,7 @@ export async function handleMergeRequestCodeReview( platformIntegrationId: integration.id, } satisfies ReviewScope; - // 4. Skip merge commits on update (e.g. merging base branch into feature branch). + // Skip merge commits on update (e.g. merging base branch into feature branch). // Runs before cancellation so that an in-flight review at an earlier SHA is preserved: // a merge commit introduces no new feature work and should not supersede the existing review. if ( @@ -217,8 +194,8 @@ export async function handleMergeRequestCodeReview( return NextResponse.json({ message: 'Skipped merge commit' }, { status: 200 }); } - // 5. Cancel any existing reviews for this MR (different SHA) - // This prevents spam when user pushes multiple commits quickly + // Cancel any existing reviews for this MR (different SHA). + // This prevents spam when user pushes multiple commits quickly. const cancelledReviews = await cancelSupersededReviewsForPR(reviewScope, headSha); if (cancelledReviews.length > 0) { @@ -239,9 +216,9 @@ export async function handleMergeRequestCodeReview( }); } - // 6. Get integration details needed for best-effort GitLab status cleanup. - // This must run before duplicate-review return so redeliveries still clean up - // stale statuses on superseded SHAs even when the new review already exists. + // Status cleanup must run before the duplicate-review return so + // redeliveries still clean up stale statuses on superseded SHAs even + // when the new review already exists. const fullIntegration = await getIntegrationById(integration.id); const metadata = fullIntegration?.metadata as { gitlab_instance_url?: string; @@ -258,7 +235,6 @@ export async function handleMergeRequestCodeReview( }); } - // 7. Check for duplicate review (same project, MR, SHA) const existingReview = await findExistingReview(reviewScope, headSha); if (existingReview) { @@ -275,10 +251,9 @@ export async function handleMergeRequestCodeReview( ); } - // 8. Resolve checkout ref (fork MRs use refs/merge-requests//head) const { checkoutRef } = resolveMergeRequestCheckoutRef(payload); - // 9. Create review record (session_id will be updated async) + // The review record's session_id is updated async. const reviewId = await createCodeReview({ owner, platformIntegrationId: integration.id, @@ -297,7 +272,6 @@ export async function handleMergeRequestCodeReview( logExceptInTest(`Created code review ${reviewId} for ${project.path_with_namespace}!${mr.iid}`); - // 10. Post 👀 reaction and set commit status (using PrAT for bot identity) if (fullIntegration) { try { const pratToken = await getOrCreateProjectAccessToken( @@ -342,7 +316,6 @@ export async function handleMergeRequestCodeReview( } } - // 11. Try to dispatch pending reviews (including this new one) // Review is created with status='pending' and dispatch will pick it up if slots available try { const dispatchResult = await tryDispatchPendingReviews(owner); @@ -367,7 +340,6 @@ export async function handleMergeRequestCodeReview( // Don't throw - review record created as pending, will be picked up later } - // 12. Return 202 Accepted (always succeeds, review queued as pending) return NextResponse.json( { message: 'Code review queued', @@ -608,9 +580,6 @@ async function migrateInFlightReviewsToMergeCommitHead(args: { } } -/** - * Main router for merge request events - */ export async function handleMergeRequest( payload: MergeRequestPayload, integration: PlatformIntegration diff --git a/apps/web/src/lib/integrations/platforms/gitlab/webhook-schemas.ts b/apps/web/src/lib/integrations/platforms/gitlab/webhook-schemas.ts index a1de2f0431..e443f151cb 100644 --- a/apps/web/src/lib/integrations/platforms/gitlab/webhook-schemas.ts +++ b/apps/web/src/lib/integrations/platforms/gitlab/webhook-schemas.ts @@ -7,9 +7,6 @@ import { z } from 'zod'; -/** - * GitLab User schema (common across events) - */ const GitLabUserSchema = z.object({ id: z.number(), name: z.string(), @@ -18,9 +15,6 @@ const GitLabUserSchema = z.object({ avatar_url: z.string().optional(), }); -/** - * GitLab Project schema (common across events) - */ const GitLabProjectSchema = z.object({ id: z.number(), name: z.string(), @@ -39,9 +33,6 @@ const GitLabProjectSchema = z.object({ http_url: z.string().optional(), }); -/** - * GitLab Repository schema - */ const GitLabRepositorySchema = z.object({ name: z.string(), url: z.string(), @@ -49,9 +40,6 @@ const GitLabRepositorySchema = z.object({ homepage: z.string().optional(), }); -/** - * GitLab Commit schema - */ const GitLabCommitSchema = z.object({ id: z.string(), message: z.string(), @@ -66,9 +54,6 @@ const GitLabCommitSchema = z.object({ .optional(), }); -/** - * GitLab Label schema - */ const GitLabLabelSchema = z.object({ id: z.number(), title: z.string(), @@ -82,9 +67,6 @@ const GitLabLabelSchema = z.object({ group_id: z.number().nullable().optional(), }); -/** - * Merge Request object attributes schema - */ const MergeRequestObjectAttributesSchema = z.object({ id: z.number(), iid: z.number(), // Internal ID - equivalent to PR number @@ -137,10 +119,6 @@ const MergeRequestObjectAttributesSchema = z.object({ first_contribution: z.boolean().optional(), }); -/** - * Merge Request Webhook Payload Schema - * Triggered when a merge request is created, updated, merged, or closed - */ export const MergeRequestPayloadSchema = z.object({ object_kind: z.literal('merge_request'), event_type: z.literal('merge_request'), @@ -183,10 +161,6 @@ export const MergeRequestPayloadSchema = z.object({ export type MergeRequestPayload = z.infer; -/** - * Push Event Webhook Payload Schema - * Triggered when commits are pushed to a repository - */ export const PushEventPayloadSchema = z.object({ object_kind: z.literal('push'), event_name: z.literal('push').optional(), @@ -224,10 +198,6 @@ export const PushEventPayloadSchema = z.object({ export type PushEventPayload = z.infer; -/** - * Note (Comment) Event Webhook Payload Schema - * Triggered when a comment is made on a commit, merge request, issue, or snippet - */ export const NoteEventPayloadSchema = z.object({ object_kind: z.literal('note'), event_type: z.literal('note'), @@ -303,9 +273,6 @@ export const NoteEventPayloadSchema = z.object({ export type NoteEventPayload = z.infer; -/** - * Pipeline Event Webhook Payload Schema (for future use) - */ export const PipelineEventPayloadSchema = z.object({ object_kind: z.literal('pipeline'), object_attributes: z.object({ diff --git a/apps/web/src/lib/integrations/platforms/gitlab/webhook-sync.ts b/apps/web/src/lib/integrations/platforms/gitlab/webhook-sync.ts index da5e091573..b22a25cbdf 100644 --- a/apps/web/src/lib/integrations/platforms/gitlab/webhook-sync.ts +++ b/apps/web/src/lib/integrations/platforms/gitlab/webhook-sync.ts @@ -1,10 +1,3 @@ -/** - * GitLab Webhook Sync - * - * Handles automatic creation and deletion of webhooks when users - * configure code reviews for their GitLab repositories. - */ - import { APP_URL } from '@/lib/constants'; import { logExceptInTest } from '@/lib/utils.server'; import { @@ -18,42 +11,27 @@ import { const DEFAULT_GITLAB_URL = 'https://gitlab.com'; /** - * Encodes a webhook URL for GitLab API. * GitLab requires special characters like colons to be percent-encoded. - * - * @param url - The webhook URL to encode - * @returns The encoded URL */ function encodeWebhookUrl(url: string): string { try { const parsed = new URL(url); - // Encode the host (which includes the port with colon) // GitLab requires the colon in "localhost:3000" to be encoded as %3A const encodedHost = encodeURIComponent(parsed.host); return `${parsed.protocol}//${encodedHost}${parsed.pathname}${parsed.search}${parsed.hash}`; } catch { - // If URL parsing fails, return the original URL return url; } } -/** - * Kilo webhook URL for GitLab (encoded for GitLab API) - */ export const KILO_GITLAB_WEBHOOK_URL = encodeWebhookUrl(`${APP_URL}/api/webhooks/gitlab`); -/** - * Configured webhook info stored in integration metadata - */ export type ConfiguredWebhook = { hook_id: number; created_at: string; updated_at?: string; }; -/** - * Result of a webhook sync operation - */ export type WebhookSyncResult = { created: Array<{ projectId: number; hookId: number }>; updated: Array<{ projectId: number; hookId: number }>; @@ -61,20 +39,6 @@ export type WebhookSyncResult = { errors: Array<{ projectId: number; error: string; operation: 'create' | 'update' | 'delete' }>; }; -/** - * Syncs webhooks for the given repositories. - * - * - Creates webhooks for newly selected repositories - * - Deletes webhooks for repositories that were removed from selection - * - Updates webhooks if they already exist but need reconfiguration - * - * @param accessToken - OAuth access token (requires Maintainer+ role on projects) - * @param webhookSecret - The webhook secret for this integration - * @param selectedRepositoryIds - Currently selected repository IDs - * @param previousRepositoryIds - Previously selected repository IDs - * @param configuredWebhooks - Map of project ID to webhook info from metadata - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - */ export async function syncWebhooksForRepositories( accessToken: string, webhookSecret: string, @@ -93,13 +57,10 @@ export async function syncWebhooksForRepositories( errors: [], }; - // Clone the configured webhooks to track updates const updatedWebhooks: Record = { ...configuredWebhooks }; - // Find repos that were added (need webhook creation) const addedRepos = selectedRepositoryIds.filter(id => !previousRepositoryIds.includes(id)); - // Find repos that were removed (need webhook deletion) const removedRepos = previousRepositoryIds.filter(id => !selectedRepositoryIds.includes(id)); logExceptInTest('[syncWebhooksForRepositories] Starting sync', { @@ -110,10 +71,8 @@ export async function syncWebhooksForRepositories( webhookUrl: KILO_GITLAB_WEBHOOK_URL, }); - // Create webhooks for added repos for (const projectId of addedRepos) { try { - // Check if webhook already exists (e.g., from a previous configuration) const existingWebhook = await findKiloWebhook( accessToken, projectId, @@ -122,7 +81,6 @@ export async function syncWebhooksForRepositories( ); if (existingWebhook) { - // Update existing webhook to ensure it has the correct secret const updated = await updateProjectWebhook( accessToken, projectId, @@ -144,7 +102,6 @@ export async function syncWebhooksForRepositories( hookId: updated.id, }); } else { - // Create new webhook const created = await createProjectWebhook( accessToken, projectId, @@ -165,7 +122,6 @@ export async function syncWebhooksForRepositories( }); } } catch (error) { - // Provide a more user-friendly error message for permission errors let errorMessage: string; if (error instanceof GitLabWebhookPermissionError) { errorMessage = `Permission denied: You need Maintainer role or higher on this project to configure webhooks automatically. You can still configure the webhook manually in GitLab.`; @@ -187,12 +143,10 @@ export async function syncWebhooksForRepositories( } } - // Delete webhooks for removed repos for (const projectId of removedRepos) { const webhookInfo = configuredWebhooks[String(projectId)]; if (!webhookInfo) { - // No webhook was configured for this project, skip logExceptInTest('[syncWebhooksForRepositories] No webhook to delete', { projectId }); continue; } @@ -215,7 +169,6 @@ export async function syncWebhooksForRepositories( operation: 'delete', }); - // Still remove from our tracking since we can't manage it delete updatedWebhooks[String(projectId)]; logExceptInTest('[syncWebhooksForRepositories] Failed to delete webhook', { @@ -236,15 +189,6 @@ export async function syncWebhooksForRepositories( return { result, updatedWebhooks }; } -/** - * Creates webhooks for all selected repositories. - * Used for initial setup when auto-configure is enabled. - * - * @param accessToken - OAuth access token (requires Maintainer+ role on projects) - * @param webhookSecret - The webhook secret for this integration - * @param repositoryIds - Repository IDs to create webhooks for - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - */ export async function createWebhooksForRepositories( accessToken: string, webhookSecret: string, @@ -267,14 +211,6 @@ export async function createWebhooksForRepositories( })); } -/** - * Deletes all configured webhooks. - * Used when disabling code reviews or disconnecting the integration. - * - * @param accessToken - OAuth access token (requires Maintainer+ role on projects) - * @param configuredWebhooks - Map of project ID to webhook info from metadata - * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - */ export async function deleteAllWebhooks( accessToken: string, configuredWebhooks: Record,