From 2549db8c5a3972115c8a0fccb4901d689b70dac8 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Wed, 1 Jul 2026 20:17:00 +0300 Subject: [PATCH] fix(code-reviews): guard retrigger active reviews --- .../[reviewId]/CodeReviewDetailClient.tsx | 72 +++++++--- .../code-reviews/CodeReviewJobsCard.tsx | 86 ++++++----- .../code-reviews/useRetriggerReview.ts | 73 ++++++++++ apps/web/src/lib/code-reviews/core/schemas.ts | 1 + .../src/routers/code-reviews-router.test.ts | 134 ++++++++++++++++++ .../code-reviews/code-reviews-router.ts | 74 +++++++++- 6 files changed, 382 insertions(+), 58 deletions(-) create mode 100644 apps/web/src/components/code-reviews/useRetriggerReview.ts diff --git a/apps/web/src/app/(app)/code-reviews/[reviewId]/CodeReviewDetailClient.tsx b/apps/web/src/app/(app)/code-reviews/[reviewId]/CodeReviewDetailClient.tsx index 6d30efeaa0..dee5c116e0 100644 --- a/apps/web/src/app/(app)/code-reviews/[reviewId]/CodeReviewDetailClient.tsx +++ b/apps/web/src/app/(app)/code-reviews/[reviewId]/CodeReviewDetailClient.tsx @@ -2,9 +2,20 @@ import { Badge } from '@/components/ui/badge'; import { Button } from '@/components/ui/button'; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from '@/components/ui/alert-dialog'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { PageContainer } from '@/components/layouts/PageContainer'; import { CodeReviewStreamView } from '@/components/code-reviews/CodeReviewStreamView'; +import { useRetriggerReview } from '@/components/code-reviews/useRetriggerReview'; import { formatTokenCount } from '@/lib/code-reviews/summary/usage-footer'; import { ExternalLink, @@ -70,21 +81,19 @@ export function CodeReviewDetailClient({ reviewId }: CodeReviewDetailClientProps }, }); - const retriggerMutation = useMutation( - trpc.codeReviews.retrigger.mutationOptions({ - onSuccess: async () => { - toast.success('Code review retriggered', { - description: 'The code review has been queued for processing.', - }); - await queryClient.invalidateQueries({ - queryKey: trpc.codeReviews.get.queryKey({ reviewId }), - }); - }, - onError: err => { - toast.error('Failed to retrigger code review', { description: err.message }); - }, - }) - ); + const { + retriggerReview, + confirmOpen, + setConfirmOpen, + confirmCancelAndRetry, + isPending: isRetriggerPending, + } = useRetriggerReview(reviewId, { + onRetriggered: async () => { + await queryClient.invalidateQueries({ + queryKey: trpc.codeReviews.get.queryKey({ reviewId }), + }); + }, + }); const cancelMutation = useMutation( trpc.codeReviews.cancel.mutationOptions({ @@ -303,14 +312,12 @@ export function CodeReviewDetailClient({ reviewId }: CodeReviewDetailClientProps )} @@ -318,6 +325,29 @@ export function CodeReviewDetailClient({ reviewId }: CodeReviewDetailClientProps + + + + Cancel previous review and retry? + + A previous review is still running for this pull request. Cancel that stale review, + then retry this one. + + + + Keep running + + {isRetriggerPending && } + Cancel and retry + + + + + {/* Session log / live stream */} {showStreamView && ( ); - // Retrigger mutation for failed/cancelled/interrupted reviews - const retriggerMutation = useMutation( - trpc.codeReviews.retrigger.mutationOptions({ - onSuccess: async () => { - toast.success('Code review retriggered', { - description: 'The code review has been queued for processing.', - }); - setActionInProgressId(null); - // Invalidate the query to refetch the list - await queryClient.invalidateQueries({ - queryKey: organizationId - ? trpc.codeReviews.listForOrganization.queryKey({ - organizationId, - limit: PAGE_SIZE, - offset, - platform, - }) - : trpc.codeReviews.listForUser.queryKey({ limit: PAGE_SIZE, offset, platform }), - }); - }, - onError: error => { - toast.error('Failed to retrigger code review', { - description: error.message, - }); - setActionInProgressId(null); - }, - }) - ); + const { + retriggerReview, + confirmOpen, + setConfirmOpen, + confirmCancelAndRetry, + isPending: isRetriggerPending, + pendingReviewId: retriggerPendingReviewId, + } = useRetriggerReview(undefined, { onRetriggered: invalidateJobsList }); // Cancel mutation for pending/queued/running reviews const cancelMutation = useMutation( @@ -623,11 +614,37 @@ export function CodeReviewJobsCard({ }) ); + const retriggerConfirmDialog = ( + + + + Cancel previous review and retry? + + A previous review is still running for this pull request. Cancel that stale review, then + retry this one. + + + + Keep running + + {isRetriggerPending && } + Cancel and retry + + + + + ); + if (isLoading) { return ( <> {renderJobsCardHeader('Loading jobs...')} {manualJobDialog} + {retriggerConfirmDialog} ); } @@ -652,6 +669,7 @@ export function CodeReviewJobsCard({ {manualJobDialog} + {retriggerConfirmDialog} ); } @@ -854,19 +872,16 @@ export function CodeReviewJobsCard({ @@ -923,6 +938,7 @@ export function CodeReviewJobsCard({ {manualJobDialog} + {retriggerConfirmDialog} ); } diff --git a/apps/web/src/components/code-reviews/useRetriggerReview.ts b/apps/web/src/components/code-reviews/useRetriggerReview.ts new file mode 100644 index 0000000000..b15c81b9af --- /dev/null +++ b/apps/web/src/components/code-reviews/useRetriggerReview.ts @@ -0,0 +1,73 @@ +import { useState } from 'react'; +import { useMutation } from '@tanstack/react-query'; +import { toast } from 'sonner'; +import { useTRPC } from '@/lib/trpc/utils'; + +type UseRetriggerReviewOptions = { + onRetriggered: () => Promise | void; +}; + +export function useRetriggerReview( + reviewId: string | undefined, + { onRetriggered }: UseRetriggerReviewOptions +) { + const trpc = useTRPC(); + const [confirmOpen, setConfirmOpenState] = useState(false); + const [confirmReviewId, setConfirmReviewId] = useState(null); + + const retriggerMutation = useMutation( + trpc.codeReviews.retrigger.mutationOptions({ + onSuccess: async (result, variables) => { + if (!result.success) { + toast.error('Failed to retrigger code review', { + description: String(result.error ?? 'Failed to retrigger code review'), + }); + return; + } + + if (result.outcome === 'confirm_cancel_active') { + setConfirmReviewId(variables.reviewId); + setConfirmOpenState(true); + return; + } + + setConfirmReviewId(null); + setConfirmOpenState(false); + toast.success('Code review retriggered', { + description: 'The code review has been queued for processing.', + }); + await onRetriggered(); + }, + onError: error => { + toast.error('Failed to retrigger code review', { description: error.message }); + }, + }) + ); + + function setConfirmOpen(open: boolean) { + setConfirmOpenState(open); + if (!open && !retriggerMutation.isPending) { + setConfirmReviewId(null); + } + } + + function retriggerReview(nextReviewId = reviewId) { + if (!nextReviewId) return; + retriggerMutation.mutate({ reviewId: nextReviewId }); + } + + function confirmCancelAndRetry() { + const nextReviewId = confirmReviewId ?? reviewId; + if (!nextReviewId) return; + retriggerMutation.mutate({ reviewId: nextReviewId, cancelActiveReview: true }); + } + + return { + retriggerReview, + confirmOpen, + setConfirmOpen, + confirmCancelAndRetry, + isPending: retriggerMutation.isPending, + pendingReviewId: retriggerMutation.variables?.reviewId ?? null, + }; +} diff --git a/apps/web/src/lib/code-reviews/core/schemas.ts b/apps/web/src/lib/code-reviews/core/schemas.ts index 6372ec2674..8f54b864f3 100644 --- a/apps/web/src/lib/code-reviews/core/schemas.ts +++ b/apps/web/src/lib/code-reviews/core/schemas.ts @@ -200,6 +200,7 @@ export const CancelCodeReviewInputSchema = z.object({ */ export const RetriggerCodeReviewInputSchema = z.object({ reviewId: z.string().uuid(), + cancelActiveReview: z.boolean().optional().default(false), }); // ============================================================================ diff --git a/apps/web/src/routers/code-reviews-router.test.ts b/apps/web/src/routers/code-reviews-router.test.ts index e4f6176b6c..e9363d2c9c 100644 --- a/apps/web/src/routers/code-reviews-router.test.ts +++ b/apps/web/src/routers/code-reviews-router.test.ts @@ -800,6 +800,9 @@ describe('codeReviewRouter attempts', () => { .where(eq(cloud_agent_code_reviews.repo_full_name, REPO)); await db.delete(cliSessions).where(eq(cliSessions.kilo_user_id, testUser.id)); await db.delete(agent_configs).where(eq(agent_configs.owned_by_user_id, testUser.id)); + await db + .delete(platform_integrations) + .where(eq(platform_integrations.owned_by_user_id, testUser.id)); mockCancelReview.mockReset(); mockTryDispatchPendingReviews.mockReset(); mockGetBlobContent.mockReset(); @@ -941,6 +944,137 @@ describe('codeReviewRouter attempts', () => { expect(mockTryDispatchPendingReviews).toHaveBeenCalled(); }); + it('blocks retrigger when a newer provider-publishing review is active', async () => { + await insertEnabledAgentConfig(); + const integration = await insertGitHubIntegration(testUser.id, 'lite'); + const [failedReview] = await db + .insert(cloud_agent_code_reviews) + .values( + reviewValues(testUser.id, 'failed', { + platform_integration_id: integration.id, + pr_number: 20, + pr_url: `https://github.com/${REPO}/pull/20`, + head_sha: 'failed-target-sha', + created_at: '2026-06-01T10:00:00.000Z', + error_message: 'Previous failure', + }) + ) + .returning({ id: cloud_agent_code_reviews.id }); + const [activeReview] = await db + .insert(cloud_agent_code_reviews) + .values( + reviewValues(testUser.id, 'running', { + platform_integration_id: integration.id, + pr_number: 20, + pr_url: `https://github.com/${REPO}/pull/20`, + head_sha: 'newer-active-sha', + created_at: '2026-06-01T11:00:00.000Z', + session_id: 'agent-active-newer', + }) + ) + .returning({ id: cloud_agent_code_reviews.id }); + + const caller = await createCallerForUser(testUser.id); + + await expect(caller.codeReviews.retrigger({ reviewId: failedReview.id })).rejects.toMatchObject( + { + code: 'CONFLICT', + message: expect.stringContaining('A newer review is already running'), + } + ); + + const reviews = await db + .select({ id: cloud_agent_code_reviews.id, status: cloud_agent_code_reviews.status }) + .from(cloud_agent_code_reviews) + .where(inArray(cloud_agent_code_reviews.id, [failedReview.id, activeReview.id])); + + expect(reviews).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: failedReview.id, status: 'failed' }), + expect.objectContaining({ id: activeReview.id, status: 'running' }), + ]) + ); + expect(mockTryDispatchPendingReviews).not.toHaveBeenCalled(); + }); + + it('confirms before cancelling an older active review and retriggering', async () => { + await insertEnabledAgentConfig(); + const integration = await insertGitHubIntegration(testUser.id, 'lite'); + const [activeReview] = await db + .insert(cloud_agent_code_reviews) + .values( + reviewValues(testUser.id, 'running', { + platform_integration_id: integration.id, + pr_number: 21, + pr_url: `https://github.com/${REPO}/pull/21`, + head_sha: 'older-active-sha', + created_at: '2026-06-01T10:00:00.000Z', + session_id: 'agent-active-older', + }) + ) + .returning({ id: cloud_agent_code_reviews.id }); + const [activeAttempt] = await db + .insert(cloud_agent_code_review_attempts) + .values({ + code_review_id: activeReview.id, + attempt_number: 1, + status: 'running', + session_id: 'agent-active-older', + }) + .returning({ id: cloud_agent_code_review_attempts.id }); + const [failedReview] = await db + .insert(cloud_agent_code_reviews) + .values( + reviewValues(testUser.id, 'failed', { + platform_integration_id: integration.id, + pr_number: 21, + pr_url: `https://github.com/${REPO}/pull/21`, + head_sha: 'failed-newer-sha', + created_at: '2026-06-01T11:00:00.000Z', + error_message: 'Previous failure', + }) + ) + .returning({ id: cloud_agent_code_reviews.id }); + + const caller = await createCallerForUser(testUser.id); + const confirmResult = await caller.codeReviews.retrigger({ reviewId: failedReview.id }); + + expect(confirmResult).toEqual( + expect.objectContaining({ + success: true, + outcome: 'confirm_cancel_active', + activeReview: { id: activeReview.id, headSha: 'older-active-sha' }, + }) + ); + expect(mockTryDispatchPendingReviews).not.toHaveBeenCalled(); + expect(mockCancelReview).not.toHaveBeenCalled(); + + const retriggerResult = await caller.codeReviews.retrigger({ + reviewId: failedReview.id, + cancelActiveReview: true, + }); + + expect(retriggerResult).toEqual({ success: true, outcome: 'retriggered' }); + expect(mockCancelReview).toHaveBeenCalledWith( + activeReview.id, + 'Superseded by manual retrigger', + activeAttempt.id + ); + expect(mockTryDispatchPendingReviews).toHaveBeenCalled(); + + const reviews = await db + .select({ id: cloud_agent_code_reviews.id, status: cloud_agent_code_reviews.status }) + .from(cloud_agent_code_reviews) + .where(inArray(cloud_agent_code_reviews.id, [failedReview.id, activeReview.id])); + + expect(reviews).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: failedReview.id, status: 'pending' }), + expect.objectContaining({ id: activeReview.id, status: 'cancelled' }), + ]) + ); + }); + it('blocks retrigger while Code Reviewer has action-required state', async () => { await insertEnabledAgentConfig({ code_review_action_required: { 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 4f8b796aff..8dd141ec64 100644 --- a/apps/web/src/routers/code-reviews/code-reviews-router.ts +++ b/apps/web/src/routers/code-reviews/code-reviews-router.ts @@ -26,6 +26,7 @@ import { createCodeReviewAttempt, getLatestCodeReviewAttempt, getSessionUsageFromBilling, + findActiveProviderPublishingReview, } from '@/lib/code-reviews/db/code-reviews'; import { getIntegrationById } from '@/lib/integrations/db/platform-integrations'; import { createCheckRun, updateCheckRun } from '@/lib/integrations/platforms/github/adapter'; @@ -204,6 +205,33 @@ async function cancelPRGateCheck(review: CloudAgentCodeReview) { } } +async function stopActiveReviewForRetrigger(review: CloudAgentCodeReview): Promise { + if (review.status === 'running' || review.status === 'queued') { + try { + const latestAttempt = await getLatestCodeReviewAttempt(review.id); + await codeReviewWorkerClient.cancelReview( + review.id, + 'Superseded by manual retrigger', + latestAttempt?.id + ); + } catch (error) { + logExceptInTest('[retrigger] Worker interrupt of stale review failed:', error); + } + } + + await cancelCodeReview(review.id); + + try { + await cancelPRGateCheck(review); + } catch (error) { + logExceptInTest('[retrigger] Failed to finalize gate for cancelled review:', error); + } +} + +function isPostgresUniqueViolation(error: unknown): boolean { + return error instanceof Error && 'code' in error && error.code === '23505'; +} + export const codeReviewRouter = createTRPCRouter({ analytics: codeReviewAnalyticsRouter, @@ -530,6 +558,36 @@ export const codeReviewRouter = createTRPCRouter({ }); } + if (review.platform_integration_id && shouldPublishCodeReviewToProvider(review)) { + const activeReview = await findActiveProviderPublishingReview({ + platformIntegrationId: review.platform_integration_id, + repoFullName: review.repo_full_name, + prNumber: review.pr_number, + }); + + if (activeReview && activeReview.id !== review.id) { + const activeIsOlder = + new Date(activeReview.created_at).getTime() < new Date(review.created_at).getTime(); + + if (!activeIsOlder) { + throw new TRPCError({ + code: 'CONFLICT', + message: + 'A newer review is already running for this pull request. Let it finish instead of retriggering this one.', + }); + } + + if (!input.cancelActiveReview) { + return successResult({ + outcome: 'confirm_cancel_active' as const, + activeReview: { id: activeReview.id, headSha: activeReview.head_sha }, + }); + } + + await stopActiveReviewForRetrigger(activeReview); + } + } + // Build owner object for dispatch. // For org reviews, use the bot user ID so retrigger dispatch matches webhook-created reviews. let owner: Owner; @@ -575,7 +633,19 @@ export const codeReviewRouter = createTRPCRouter({ const currentAttempt = await ensureCurrentCodeReviewAttemptFromReview(review); // Reset the review for retry - await resetCodeReviewForRetry(input.reviewId); + try { + await resetCodeReviewForRetry(input.reviewId); + } catch (error) { + if (isPostgresUniqueViolation(error)) { + throw new TRPCError({ + code: 'CONFLICT', + message: + 'Another review just started for this pull request. Please try again in a moment.', + }); + } + + throw error; + } await createCodeReviewAttempt({ codeReviewId: input.reviewId, retryOfAttemptId: currentAttempt.id, @@ -594,7 +664,7 @@ export const codeReviewRouter = createTRPCRouter({ // Try to dispatch the review await tryDispatchPendingReviews(owner); - return successResult({ message: 'Code review retriggered successfully' }); + return successResult({ outcome: 'retriggered' as const }); } catch (error) { if (error instanceof TRPCError) { throw error;