Skip to content

Commit 53c5ad8

Browse files
alex-alecualex-alecu
authored
fix(code-review): disable code review on bad config (#3558)
* fix(code-review): disable on action required * fix(code-review): fail missing config * refactor(code-review): return email flag * fix(code-review): release failed claim * fix(code-review): skip repeat disable * fix(code-review): record sent email * fix(worker): accept terminal reasons
1 parent 986b03d commit 53c5ad8

29 files changed

Lines changed: 1899 additions & 103 deletions

File tree

‎apps/web/src/app/(app)/code-reviews/ReviewAgentPageClient.tsx‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { useEffect } from 'react';
44
import { toast } from 'sonner';
55
import { ReviewConfigForm } from '@/components/code-reviews/ReviewConfigForm';
6+
import { CodeReviewActionRequiredAlert } from '@/components/code-reviews/CodeReviewActionRequiredAlert';
67
import { CodeReviewJobsCard } from '@/components/code-reviews/CodeReviewJobsCard';
78
import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert';
89
import { Badge } from '@/components/ui/badge';
@@ -56,6 +57,11 @@ export function ReviewAgentPageClient({
5657
trpc.personalReviewAgent.getGitLabStatus.queryOptions()
5758
);
5859

60+
const { data: selectedConfigData } = useQuery(
61+
trpc.personalReviewAgent.getReviewConfig.queryOptions({ platform: selectedPlatform })
62+
);
63+
const selectedActionRequired = selectedConfigData?.actionRequired ?? null;
64+
5965
const isGitHubAppInstalled =
6066
githubStatusData?.connected && githubStatusData?.integration?.isValid;
6167
const isGitLabConnected = gitlabStatusData?.connected && gitlabStatusData?.integration?.isValid;
@@ -151,6 +157,10 @@ export function ReviewAgentPageClient({
151157
</Alert>
152158
)}
153159

160+
{selectedPlatform === 'github' && selectedActionRequired && (
161+
<CodeReviewActionRequiredAlert actionRequired={selectedActionRequired} />
162+
)}
163+
154164
{/* GitHub Configuration Tabs */}
155165
<Tabs defaultValue="config" className="w-full">
156166
<TabsList className="grid w-full max-w-2xl grid-cols-2">
@@ -211,6 +221,10 @@ export function ReviewAgentPageClient({
211221
</Alert>
212222
)}
213223

224+
{selectedPlatform === 'gitlab' && selectedActionRequired && (
225+
<CodeReviewActionRequiredAlert actionRequired={selectedActionRequired} />
226+
)}
227+
214228
{/* GitLab Configuration Tabs */}
215229
<Tabs defaultValue="config" className="w-full">
216230
<TabsList className="grid w-full max-w-2xl grid-cols-2">

‎apps/web/src/app/(app)/organizations/[id]/code-reviews/ReviewAgentPageClient.tsx‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { useEffect } from 'react';
44
import { toast } from 'sonner';
55
import { ReviewConfigForm } from '@/components/code-reviews/ReviewConfigForm';
6+
import { CodeReviewActionRequiredAlert } from '@/components/code-reviews/CodeReviewActionRequiredAlert';
67
import { CodeReviewJobsCard } from '@/components/code-reviews/CodeReviewJobsCard';
78
import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert';
89
import { Badge } from '@/components/ui/badge';
@@ -63,6 +64,14 @@ export function ReviewAgentPageClient({
6364
})
6465
);
6566

67+
const { data: selectedConfigData } = useQuery(
68+
trpc.organizations.reviewAgent.getReviewConfig.queryOptions({
69+
organizationId,
70+
platform: selectedPlatform,
71+
})
72+
);
73+
const selectedActionRequired = selectedConfigData?.actionRequired ?? null;
74+
6675
const isGitHubAppInstalled =
6776
githubStatusData?.connected && githubStatusData?.integration?.isValid;
6877
const isGitLabConnected = gitlabStatusData?.connected && gitlabStatusData?.integration?.isValid;
@@ -158,6 +167,13 @@ export function ReviewAgentPageClient({
158167
</Alert>
159168
)}
160169

170+
{selectedPlatform === 'github' && selectedActionRequired && (
171+
<CodeReviewActionRequiredAlert
172+
actionRequired={selectedActionRequired}
173+
organizationId={organizationId}
174+
/>
175+
)}
176+
161177
{/* GitHub Configuration Tabs */}
162178
<Tabs defaultValue="config" className="w-full">
163179
<TabsList className="grid w-full max-w-2xl grid-cols-2">
@@ -218,6 +234,13 @@ export function ReviewAgentPageClient({
218234
</Alert>
219235
)}
220236

237+
{selectedPlatform === 'gitlab' && selectedActionRequired && (
238+
<CodeReviewActionRequiredAlert
239+
actionRequired={selectedActionRequired}
240+
organizationId={organizationId}
241+
/>
242+
)}
243+
221244
{/* GitLab Configuration Tabs */}
222245
<Tabs defaultValue="config" className="w-full">
223246
<TabsList className="grid w-full max-w-2xl grid-cols-2">

‎apps/web/src/app/admin/components/CodeReviewErrorAnalysis.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ type Props = {
4949
};
5050

5151
const CATEGORY_COLORS: Record<string, string> = {
52+
'Action Required': 'bg-yellow-500',
5253
'Rate Limited': 'bg-amber-500',
5354
Timeout: 'bg-orange-500',
5455
'Context Window Exceeded': 'bg-purple-500',

‎apps/web/src/app/api/internal/code-review-status/[reviewId]/route.test.ts‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ const mockCaptureMessage = jest.fn<any>();
7474
const mockAppendReviewSummaryFooter = jest.fn<any>();
7575
// eslint-disable-next-line @typescript-eslint/no-explicit-any
7676
const mockRetryReviewFresh = jest.fn<any>();
77+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
78+
const mockDisableCodeReviewForActionRequiredFailure = jest.fn<any>();
7779

7880
// --- Module mocks ---
7981

@@ -142,6 +144,15 @@ jest.mock('@/lib/code-reviews/summary/usage-footer', () => ({
142144
appendReviewSummaryFooter: (...args: unknown[]) => mockAppendReviewSummaryFooter(...args),
143145
}));
144146

147+
jest.mock('@/lib/code-reviews/action-required', () => {
148+
const actual = jest.requireActual<Record<string, unknown>>('@/lib/code-reviews/action-required');
149+
return {
150+
...actual,
151+
disableCodeReviewForActionRequiredFailure: (...args: unknown[]) =>
152+
mockDisableCodeReviewForActionRequiredFailure(...args),
153+
};
154+
});
155+
145156
jest.mock('@/lib/constants', () => ({
146157
APP_URL: 'https://test.kilo.ai',
147158
}));
@@ -334,6 +345,7 @@ beforeEach(async () => {
334345
mockUpdateCodeReviewUsage.mockResolvedValue(undefined);
335346
mockUpdateCodeReviewStatusIfNonTerminal.mockResolvedValue(true);
336347
mockAppendReviewSummaryFooter.mockReturnValue('body with footer');
348+
mockDisableCodeReviewForActionRequiredFailure.mockResolvedValue(undefined);
337349
({ POST } = await import('./route'));
338350
});
339351

@@ -479,6 +491,96 @@ describe('POST /api/internal/code-review-status/[reviewId]', () => {
479491
);
480492
});
481493

494+
it('infers BYOK invalid-key callbacks as action-required failures', async () => {
495+
mockGetCodeReviewById.mockResolvedValue(makeReview());
496+
497+
const response = await POST(
498+
makeRequest({
499+
status: 'failed',
500+
errorMessage:
501+
'[BYOK] Your API key is invalid or has been revoked. Please check your API key configuration.',
502+
}),
503+
makeParams(REVIEW_ID)
504+
);
505+
506+
expect(response.status).toBe(200);
507+
expect(mockUpdateCodeReviewStatus).toHaveBeenCalledWith(
508+
REVIEW_ID,
509+
'failed',
510+
expect.objectContaining({
511+
terminalReason: 'byok_invalid_key',
512+
})
513+
);
514+
expect(mockCreateInfraRetryAttemptIfMissing).not.toHaveBeenCalled();
515+
expect(mockRetryReviewFresh).not.toHaveBeenCalled();
516+
expect(mockDisableCodeReviewForActionRequiredFailure).toHaveBeenCalledWith(
517+
expect.objectContaining({
518+
owner: { type: 'user', id: 'user-1', userId: 'user-1' },
519+
platform: 'github',
520+
reviewId: REVIEW_ID,
521+
reason: 'byok_invalid_key',
522+
})
523+
);
524+
expect(mockUpdateCheckRun).toHaveBeenCalledWith(
525+
'inst-1',
526+
'owner',
527+
'repo',
528+
12345,
529+
expect.objectContaining({
530+
conclusion: 'action_required',
531+
output: expect.objectContaining({ title: 'BYOK API key needs attention' }),
532+
}),
533+
'standard'
534+
);
535+
});
536+
537+
it('infers GitHub installation and IP allow-list callback failures', async () => {
538+
mockGetCodeReviewById.mockResolvedValue(makeReview());
539+
540+
await POST(
541+
makeRequest({
542+
status: 'failed',
543+
errorMessage:
544+
'Dispatch failed: GitHub token or active app installation required for this repository (no_installation_found)',
545+
}),
546+
makeParams(REVIEW_ID)
547+
);
548+
549+
expect(mockUpdateCodeReviewStatus).toHaveBeenLastCalledWith(
550+
REVIEW_ID,
551+
'failed',
552+
expect.objectContaining({ terminalReason: 'github_installation_required' })
553+
);
554+
555+
jest.clearAllMocks();
556+
mockGetCodeReviewById.mockResolvedValue(makeReview());
557+
mockUpdateCodeReviewAttemptForCallback.mockImplementation(async params =>
558+
makeAttempt({
559+
status: params.status,
560+
error_message: params.errorMessage ?? null,
561+
terminal_reason: params.terminalReason ?? null,
562+
})
563+
);
564+
mockGetLatestCodeReviewAttempt.mockResolvedValue(makeAttempt());
565+
mockGetIntegrationById.mockResolvedValue(makeIntegration());
566+
mockDisableCodeReviewForActionRequiredFailure.mockResolvedValue(undefined);
567+
568+
await POST(
569+
makeRequest({
570+
status: 'failed',
571+
errorMessage:
572+
'Although you appear to have the correct authorization credentials, the `acme` organization has an IP allow list enabled, and 192.0.2.1 is not permitted.',
573+
}),
574+
makeParams(REVIEW_ID)
575+
);
576+
577+
expect(mockUpdateCodeReviewStatus).toHaveBeenLastCalledWith(
578+
REVIEW_ID,
579+
'failed',
580+
expect.objectContaining({ terminalReason: 'github_ip_allow_list' })
581+
);
582+
});
583+
482584
it('keeps interrupted non-billing callbacks as cancelled', async () => {
483585
mockGetCodeReviewById.mockResolvedValue(makeReview());
484586

0 commit comments

Comments
 (0)