From 296d91f7d875db922ed709ee5ec9a810761eab56 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Thu, 7 May 2026 09:15:55 +0300 Subject: [PATCH] fix(github): use app type for check run updates --- .../[reviewId]/route.test.ts | 61 ++++++++---- .../code-review-status/[reviewId]/route.ts | 3 +- .../pull-request-handler.test.ts | 98 +++++++++++++++++++ .../webhook-handlers/pull-request-handler.ts | 6 +- .../src/routers/code-reviews-router.test.ts | 56 ++++++++++- .../code-reviews/code-reviews-router.ts | 7 +- 6 files changed, 208 insertions(+), 23 deletions(-) diff --git a/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.test.ts b/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.test.ts index bd3b5b89ca..d2d33e2d45 100644 --- a/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.test.ts +++ b/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, jest, beforeEach } from '@jest/globals'; import type { NextRequest } from 'next/server'; import type * as codeReviewsDbModule from '@/lib/code-reviews/db/code-reviews'; import type * as platformIntegrationsModule from '@/lib/integrations/db/platform-integrations'; -import type { CloudAgentCodeReview } from '@kilocode/db/schema'; +import type { CloudAgentCodeReview, PlatformIntegration } from '@kilocode/db/schema'; // --- Mock functions --- @@ -173,18 +173,8 @@ function makeReview(overrides: Partial = {}): CloudAgentCo }; } -// --- Tests --- - -import type { POST as POSTType } from './route'; - -let POST: typeof POSTType; - -beforeEach(async () => { - jest.clearAllMocks(); - mockUpdateCodeReviewStatus.mockResolvedValue(undefined); - mockTryDispatchPendingReviews.mockResolvedValue(undefined); - mockGetBotUserId.mockResolvedValue(null); - mockGetIntegrationById.mockResolvedValue({ +function makeIntegration(overrides: Partial = {}): PlatformIntegration { + return { id: 'int-1', platform_installation_id: 'inst-1', platform: 'github', @@ -209,7 +199,22 @@ beforeEach(async () => { installed_at: '2025-01-01T00:00:00Z', created_at: '2025-01-01T00:00:00Z', updated_at: '2025-01-01T00:00:00Z', - }); + ...overrides, + }; +} + +// --- Tests --- + +import type { POST as POSTType } from './route'; + +let POST: typeof POSTType; + +beforeEach(async () => { + jest.clearAllMocks(); + mockUpdateCodeReviewStatus.mockResolvedValue(undefined); + mockTryDispatchPendingReviews.mockResolvedValue(undefined); + mockGetBotUserId.mockResolvedValue(null); + mockGetIntegrationById.mockResolvedValue(makeIntegration()); mockUpdateCheckRun.mockResolvedValue(undefined); mockAddReactionToPR.mockResolvedValue(undefined); mockCreatePRComment.mockResolvedValue(undefined); @@ -407,7 +412,8 @@ describe('POST /api/internal/code-review-status/[reviewId]', () => { title: 'Insufficient credits to run review', summary: 'Review could not start because the account has insufficient credits.', }), - }) + }), + 'standard' ); }); @@ -433,7 +439,27 @@ describe('POST /api/internal/code-review-status/[reviewId]', () => { output: expect.objectContaining({ title: 'Kilo Code Review failed', }), - }) + }), + 'standard' + ); + }); + + it('passes the integration GitHub app type to check run updates', async () => { + mockGetCodeReviewById.mockResolvedValue(makeReview()); + mockGetIntegrationById.mockResolvedValue(makeIntegration({ github_app_type: 'lite' })); + + await POST(makeRequest({ status: 'running' }), makeParams(REVIEW_ID)); + + expect(mockUpdateCheckRun).toHaveBeenCalledWith( + 'inst-1', + 'owner', + 'repo', + 12345, + expect.objectContaining({ + status: 'in_progress', + conclusion: undefined, + }), + 'lite' ); }); @@ -458,7 +484,8 @@ describe('POST /api/internal/code-review-status/[reviewId]', () => { output: expect.objectContaining({ title: 'Insufficient credits to run review', }), - }) + }), + 'standard' ); }); }); diff --git a/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.ts b/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.ts index e9409ef362..37a36cfd9c 100644 --- a/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.ts +++ b/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.ts @@ -399,7 +399,8 @@ async function updatePRGateCheck( title: checkRunMapping.title, summary: checkRunMapping.summary, }, - } + }, + integration.github_app_type ?? 'standard' ); logExceptInTest( 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 a59985cd92..8d28a549c1 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 @@ -1,5 +1,16 @@ const mockGetBotUserId = jest.fn(); const mockGetAgentConfigForOwner = jest.fn(); +const mockCreateCheckRun = jest.fn(); +const mockUpdateCheckRun = jest.fn(); +const mockUpdateCheckRunId = jest.fn(); +const mockCreateCodeReview = jest.fn(); +const mockFindExistingReview = jest.fn(); +const mockFindActiveReviewsForPR = jest.fn(); +const mockUpdateReviewHeadShaAndCheckRun = jest.fn(); +const mockTryDispatchPendingReviews = jest.fn(); +const mockCancelReview = jest.fn(); +const mockAddReactionToPR = jest.fn(); +const mockIsMergeCommit = jest.fn(); jest.mock('@/lib/bot-users/bot-user-service', () => ({ getBotUserId: (organizationId: string, botType: string) => @@ -11,6 +22,32 @@ jest.mock('@/lib/agent-config/db/agent-configs', () => ({ mockGetAgentConfigForOwner(owner, agentType, platform), })); +jest.mock('@/lib/code-reviews/db/code-reviews', () => ({ + createCodeReview: (...args: unknown[]) => mockCreateCodeReview(...args), + findExistingReview: (...args: unknown[]) => mockFindExistingReview(...args), + findActiveReviewsForPR: (...args: unknown[]) => mockFindActiveReviewsForPR(...args), + updateReviewHeadShaAndCheckRun: (...args: unknown[]) => + mockUpdateReviewHeadShaAndCheckRun(...args), + updateCheckRunId: (...args: unknown[]) => mockUpdateCheckRunId(...args), +})); + +jest.mock('@/lib/code-reviews/dispatch/dispatch-pending-reviews', () => ({ + tryDispatchPendingReviews: (...args: unknown[]) => mockTryDispatchPendingReviews(...args), +})); + +jest.mock('@/lib/code-reviews/client/code-review-worker-client', () => ({ + codeReviewWorkerClient: { + cancelReview: (...args: unknown[]) => mockCancelReview(...args), + }, +})); + +jest.mock('@/lib/integrations/platforms/github/adapter', () => ({ + addReactionToPR: (...args: unknown[]) => mockAddReactionToPR(...args), + createCheckRun: (...args: unknown[]) => mockCreateCheckRun(...args), + isMergeCommit: (...args: unknown[]) => mockIsMergeCommit(...args), + updateCheckRun: (...args: unknown[]) => mockUpdateCheckRun(...args), +})); + import { resolvePullRequestCheckoutRef } from '@/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref'; import { handlePullRequest, @@ -59,6 +96,17 @@ beforeEach(() => { jest.clearAllMocks(); mockGetBotUserId.mockResolvedValue(null); mockGetAgentConfigForOwner.mockResolvedValue(null); + mockCreateCheckRun.mockResolvedValue(98765); + mockUpdateCheckRun.mockResolvedValue(undefined); + mockUpdateCheckRunId.mockResolvedValue(undefined); + mockCreateCodeReview.mockResolvedValue('review-1'); + mockFindExistingReview.mockResolvedValue(null); + mockFindActiveReviewsForPR.mockResolvedValue([]); + mockUpdateReviewHeadShaAndCheckRun.mockResolvedValue(undefined); + mockTryDispatchPendingReviews.mockResolvedValue({ dispatched: 0, pending: 1, activeCount: 0 }); + mockCancelReview.mockResolvedValue({ success: true, reviewId: 'old-review' }); + mockAddReactionToPR.mockResolvedValue(undefined); + mockIsMergeCommit.mockResolvedValue(false); }); describe('resolvePullRequestCheckoutRef', () => { @@ -198,4 +246,54 @@ describe('handlePullRequest', () => { ); expect(mockGetAgentConfigForOwner).not.toHaveBeenCalled(); }); + + it('passes the integration GitHub app type when cancelling an orphaned check run', async () => { + mockGetBotUserId.mockResolvedValue('bot-user-1'); + mockGetAgentConfigForOwner.mockResolvedValue({ + is_enabled: true, + config: {}, + }); + mockUpdateCheckRunId.mockRejectedValue(new Error('database write failed')); + + const response = await handlePullRequest( + pullRequestPayload(), + platformIntegration({ github_app_type: 'standard' }) + ); + + expect(response.status).toBe(202); + expect(mockUpdateCheckRun).toHaveBeenCalledWith( + '98765', + 'acme', + 'widgets', + 98765, + { status: 'completed', conclusion: 'cancelled' }, + 'standard' + ); + }); + + it('passes the integration GitHub app type when cancelling an orphaned merge-commit check run', async () => { + mockGetBotUserId.mockResolvedValue('bot-user-1'); + mockGetAgentConfigForOwner.mockResolvedValue({ + is_enabled: true, + config: {}, + }); + mockFindActiveReviewsForPR.mockResolvedValue(['review-1']); + mockUpdateReviewHeadShaAndCheckRun.mockRejectedValue(new Error('database write failed')); + mockIsMergeCommit.mockResolvedValue(true); + + const response = await handlePullRequest( + pullRequestPayload(), + platformIntegration({ github_app_type: 'standard' }) + ); + + expect(response.status).toBe(200); + expect(mockUpdateCheckRun).toHaveBeenCalledWith( + '98765', + 'acme', + 'widgets', + 98765, + { status: 'completed', conclusion: 'cancelled' }, + 'standard' + ); + }); }); 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 b7a22a19cd..ef40c9c95b 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 @@ -299,7 +299,8 @@ export async function handlePullRequestCodeReview( repoOwner, repoName, checkRunId, - { status: 'completed', conclusion: 'cancelled' } + { status: 'completed', conclusion: 'cancelled' }, + appType ); logExceptInTest( `Cancelled orphaned check run ${checkRunId} for ${repository.full_name}#${pull_request.number}` @@ -477,7 +478,8 @@ async function migrateInFlightReviewsToMergeCommitHead(args: { args.baseOwner, args.baseRepoName, newCheckRunId, - { status: 'completed', conclusion: 'cancelled' } + { status: 'completed', conclusion: 'cancelled' }, + args.appType ); } catch (cancelError) { logExceptInTest('Failed to cancel orphaned merge-commit check run:', cancelError); diff --git a/apps/web/src/routers/code-reviews-router.test.ts b/apps/web/src/routers/code-reviews-router.test.ts index 31fcf05b41..8d01f40725 100644 --- a/apps/web/src/routers/code-reviews-router.test.ts +++ b/apps/web/src/routers/code-reviews-router.test.ts @@ -16,14 +16,21 @@ jest.mock('@/lib/integrations/platforms/gitlab/adapter', () => ({ })); import { db } from '@/lib/drizzle'; +import { updateCheckRun } from '@/lib/integrations/platforms/github/adapter'; import { createCallerForUser } from '@/routers/test-utils'; import { insertTestUser } from '@/tests/helpers/user.helper'; -import { cloud_agent_code_reviews, kilocode_users, type User } from '@kilocode/db/schema'; +import { + cloud_agent_code_reviews, + kilocode_users, + platform_integrations, + type User, +} from '@kilocode/db/schema'; import { eq } from 'drizzle-orm'; const REPO = `test-org/code-reviews-cancel-${Date.now()}`; type ReviewStatus = 'pending' | 'queued' | 'running'; type CodeReviewInsert = typeof cloud_agent_code_reviews.$inferInsert; +const mockUpdateCheckRun = jest.mocked(updateCheckRun); function reviewValues( userId: string, @@ -50,6 +57,21 @@ function reviewValues( } satisfies CodeReviewInsert; } +async function insertGitHubIntegration(userId: string, githubAppType: 'standard' | 'lite') { + const [integration] = await db + .insert(platform_integrations) + .values({ + owned_by_user_id: userId, + platform: 'github', + integration_type: 'app', + platform_installation_id: `inst-${crypto.randomUUID()}`, + github_app_type: githubAppType, + }) + .returning(); + + return integration; +} + describe('codeReviewRouter.cancel', () => { let testUser: User; @@ -59,13 +81,18 @@ describe('codeReviewRouter.cancel', () => { beforeEach(() => { mockCancelReview.mockResolvedValue({ success: true, reviewId: 'unused' }); + mockUpdateCheckRun.mockResolvedValue(undefined); }); afterEach(async () => { await db .delete(cloud_agent_code_reviews) .where(eq(cloud_agent_code_reviews.repo_full_name, REPO)); + await db + .delete(platform_integrations) + .where(eq(platform_integrations.owned_by_user_id, testUser.id)); mockCancelReview.mockReset(); + mockUpdateCheckRun.mockReset(); }); afterAll(async () => { @@ -187,4 +214,31 @@ describe('codeReviewRouter.cancel', () => { expect(storedReview?.status).toBe('running'); expect(storedReview?.completed_at).toBeNull(); }); + + it('passes the integration GitHub app type when cancelling a pending check run', async () => { + const integration = await insertGitHubIntegration(testUser.id, 'lite'); + const [review] = await db + .insert(cloud_agent_code_reviews) + .values( + reviewValues(testUser.id, 'pending', { + platform_integration_id: integration.id, + check_run_id: 12345, + }) + ) + .returning({ id: cloud_agent_code_reviews.id }); + const [repoOwner, repoName] = REPO.split('/'); + + const caller = await createCallerForUser(testUser.id); + const result = await caller.codeReviews.cancel({ reviewId: review.id }); + + expect(result.success).toBe(true); + expect(mockUpdateCheckRun).toHaveBeenCalledWith( + integration.platform_installation_id, + repoOwner, + repoName, + 12345, + expect.objectContaining({ status: 'completed', conclusion: 'cancelled' }), + 'lite' + ); + }); }); diff --git a/apps/web/src/routers/code-reviews/code-reviews-router.ts b/apps/web/src/routers/code-reviews/code-reviews-router.ts index ab2a69bbd3..2125b9e408 100644 --- a/apps/web/src/routers/code-reviews/code-reviews-router.ts +++ b/apps/web/src/routers/code-reviews/code-reviews-router.ts @@ -94,7 +94,8 @@ async function recreatePRGateCheck(review: CloudAgentCodeReview) { repoOwner, repoName, checkRunId, - { status: 'completed', conclusion: 'cancelled' } + { status: 'completed', conclusion: 'cancelled' }, + appType ); logExceptInTest( `[retrigger] Cancelled orphaned check run ${checkRunId} for ${review.repo_full_name}#${review.pr_number}` @@ -146,6 +147,7 @@ async function cancelPRGateCheck(review: CloudAgentCodeReview) { if (platform === 'github' && integration.platform_installation_id) { if (!review.check_run_id) return; + const appType = integration.github_app_type ?? 'standard'; const [repoOwner, repoName] = review.repo_full_name.split('/'); await updateCheckRun( integration.platform_installation_id, @@ -157,7 +159,8 @@ async function cancelPRGateCheck(review: CloudAgentCodeReview) { conclusion: 'cancelled', detailsUrl, output: { title: 'Kilo Code Review cancelled', summary: 'Review was cancelled.' }, - } + }, + appType ); logExceptInTest( `[cancel] Finalized check run for ${review.repo_full_name}#${review.pr_number}`