Skip to content

Commit bc0497a

Browse files
fix(security-agent): retry queued manual analysis (#3692)
1 parent 73ea1e2 commit bc0497a

5 files changed

Lines changed: 107 additions & 19 deletions

File tree

apps/web/src/components/security-agent/FindingDetailDialog.tsx

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use client';
22

3+
import { useEffect, useState } from 'react';
34
import {
45
Dialog,
56
DialogContent,
@@ -34,6 +35,9 @@ import { manualAnalysisAdmissionCopy } from './manual-analysis-admission-copy';
3435

3536
type Severity = 'critical' | 'high' | 'medium' | 'low';
3637

38+
const ACCEPTED_QUEUE_POLL_INTERVAL_MS = 3000;
39+
const ACCEPTED_QUEUE_POLL_TIMEOUT_MS = 18_000;
40+
3741
function isSeverity(value: string): value is Severity {
3842
return ['critical', 'high', 'medium', 'low'].includes(value);
3943
}
@@ -79,12 +83,16 @@ export function FindingDetailDialog({
7983
const trpc = useTRPC();
8084
const queryClient = useQueryClient();
8185
const isOrg = !!organizationId;
86+
const [queuedFindingId, setQueuedFindingId] = useState<string | null>(null);
87+
const isAwaitingAnalysisStart = queuedFindingId === finding?.id;
8288

83-
// Poll for analysis status when running.
89+
// Poll while queued admission is crossing the Worker boundary or analysis is active.
8490
// Two separate queries for org/personal to avoid type-union issues with useQuery.
8591
const pollWhileActive = (query: { state: { data?: { status?: string | null } } }) => {
8692
const status = query.state.data?.status;
87-
if (status === 'pending' || status === 'running') return 3000;
93+
if (isAwaitingAnalysisStart || status === 'pending' || status === 'running') {
94+
return ACCEPTED_QUEUE_POLL_INTERVAL_MS;
95+
}
8896
return false as const;
8997
};
9098
const orgAnalysisQuery = useQuery({
@@ -104,13 +112,31 @@ export function FindingDetailDialog({
104112
});
105113
const analysisData = isOrg ? orgAnalysisQuery.data : personalAnalysisQuery.data;
106114

115+
useEffect(() => {
116+
if (!queuedFindingId) return;
117+
const intervalId = window.setInterval(() => {
118+
void queryClient.invalidateQueries();
119+
}, ACCEPTED_QUEUE_POLL_INTERVAL_MS);
120+
const timeoutId = window.setTimeout(() => {
121+
setQueuedFindingId(current => (current === queuedFindingId ? null : current));
122+
}, ACCEPTED_QUEUE_POLL_TIMEOUT_MS);
123+
return () => {
124+
window.clearInterval(intervalId);
125+
window.clearTimeout(timeoutId);
126+
};
127+
}, [queryClient, queuedFindingId]);
128+
107129
// Start analysis mutation (organization)
108130
const startOrgAnalysisMutation = useMutation(
109131
trpc.organizations.securityAgent.startAnalysis.mutationOptions({
110132
onSuccess: async () => {
111133
toast.success(manualAnalysisAdmissionCopy.successTitle);
112134
await queryClient.invalidateQueries();
113135
},
136+
onError: (error, variables) => {
137+
toast.error(manualAnalysisAdmissionCopy.failureTitle, { description: error.message });
138+
setQueuedFindingId(current => (current === variables.findingId ? null : current));
139+
},
114140
})
115141
);
116142

@@ -121,6 +147,10 @@ export function FindingDetailDialog({
121147
toast.success(manualAnalysisAdmissionCopy.successTitle);
122148
await queryClient.invalidateQueries();
123149
},
150+
onError: (error, variables) => {
151+
toast.error(manualAnalysisAdmissionCopy.failureTitle, { description: error.message });
152+
setQueuedFindingId(current => (current === variables.findingId ? null : current));
153+
},
124154
})
125155
);
126156

@@ -135,9 +165,13 @@ export function FindingDetailDialog({
135165
const cliSessionId = analysisData?.cliSessionId ?? finding.cli_session_id;
136166

137167
const isAnalyzing =
138-
startAnalysisMutation.isPending || analysisStatus === 'pending' || analysisStatus === 'running';
168+
isAwaitingAnalysisStart ||
169+
startAnalysisMutation.isPending ||
170+
analysisStatus === 'pending' ||
171+
analysisStatus === 'running';
139172

140173
const handleStartAnalysis = ({ retrySandboxOnly }: { retrySandboxOnly?: boolean } = {}) => {
174+
setQueuedFindingId(finding.id);
141175
if (isOrg) {
142176
if (!organizationId) return;
143177
startOrgAnalysisMutation.mutate({
@@ -335,12 +369,16 @@ export function FindingDetailDialog({
335369
/>
336370
)}
337371
</div>
338-
) : analysisStatus === 'running' || analysisStatus === 'pending' ? (
372+
) : isAwaitingAnalysisStart ||
373+
analysisStatus === 'running' ||
374+
analysisStatus === 'pending' ? (
339375
<div className="rounded-lg border border-yellow-500/30 bg-yellow-500/10 p-3">
340376
<div className="flex items-center gap-2">
341377
<Loader2 className="h-4 w-4 animate-spin text-yellow-400" />
342378
<p className="text-sm text-yellow-400">
343-
{analysisStatus === 'pending' ? 'Queued...' : 'Triage in progress...'}
379+
{isAwaitingAnalysisStart || analysisStatus === 'pending'
380+
? `${manualAnalysisAdmissionCopy.pendingLabel}...`
381+
: 'Triage in progress...'}
344382
</p>
345383
</div>
346384
</div>
@@ -463,13 +501,15 @@ export function FindingDetailDialog({
463501
<div className="space-y-4">
464502
<MarkdownProse markdown={analysis.rawMarkdown} className="text-muted-foreground" />
465503
</div>
466-
) : analysisStatus === 'running' || analysisStatus === 'pending' ? (
504+
) : isAwaitingAnalysisStart ||
505+
analysisStatus === 'running' ||
506+
analysisStatus === 'pending' ? (
467507
<div className="rounded-lg border border-yellow-500/30 bg-yellow-500/10 p-3">
468508
<div className="flex items-center gap-2">
469509
<Loader2 className="h-4 w-4 animate-spin text-yellow-400" />
470510
<p className="text-sm text-yellow-400">
471-
{analysisStatus === 'pending'
472-
? 'Queued...'
511+
{isAwaitingAnalysisStart || analysisStatus === 'pending'
512+
? `${manualAnalysisAdmissionCopy.pendingLabel}...`
473513
: 'Codebase analysis in progress...'}
474514
</p>
475515
</div>

services/security-auto-analysis/src/db/queries.integration.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,57 @@ describe('security analysis durable database invariants', () => {
137137
]);
138138
});
139139

140+
it('lets manual retry supersede an unclaimed scheduled retry', async () => {
141+
const findingId = await insertFinding('manual-supersedes-scheduled-retry', 'failed');
142+
const finding = await getSecurityFindingById(client.db as never, findingId);
143+
expect(finding).not.toBeNull();
144+
if (!finding) return;
145+
146+
await client.db.insert(security_analysis_queue).values({
147+
finding_id: findingId,
148+
owned_by_user_id: testUserId,
149+
queue_status: 'queued',
150+
severity_rank: 1,
151+
queued_at: '2026-05-18T08:00:00.000Z',
152+
attempt_count: 2,
153+
next_retry_at: '2026-05-18T09:00:00.000Z',
154+
failure_code: 'NETWORK_TIMEOUT',
155+
last_error_redacted: 'prior scheduled failure',
156+
});
157+
158+
await expect(
159+
ensureManualAnalysisQueueRow(client.db as never, {
160+
finding,
161+
claimToken: 'manual-claim-token',
162+
jobId: 'manual-job',
163+
})
164+
).resolves.toBe(true);
165+
166+
const queueRows = await client.db
167+
.select({
168+
queueStatus: security_analysis_queue.queue_status,
169+
claimToken: security_analysis_queue.claim_token,
170+
claimedByJobId: security_analysis_queue.claimed_by_job_id,
171+
failureCode: security_analysis_queue.failure_code,
172+
lastErrorRedacted: security_analysis_queue.last_error_redacted,
173+
attemptCount: security_analysis_queue.attempt_count,
174+
nextRetryAt: security_analysis_queue.next_retry_at,
175+
})
176+
.from(security_analysis_queue)
177+
.where(eq(security_analysis_queue.finding_id, findingId));
178+
expect(queueRows).toEqual([
179+
{
180+
queueStatus: 'pending',
181+
claimToken: 'manual-claim-token',
182+
claimedByJobId: 'manual-job',
183+
failureCode: null,
184+
lastErrorRedacted: null,
185+
attemptCount: 0,
186+
nextRetryAt: null,
187+
},
188+
]);
189+
});
190+
140191
it('requeues stale pending rows and terminalizes stale running rows in real SQL', async () => {
141192
const pendingFindingId = await insertFinding('stale-pending');
142193
const runningFindingId = await insertFinding('stale-running', 'running');

services/security-auto-analysis/src/db/queries.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -377,10 +377,9 @@ export async function ensureManualAnalysisQueueRow(
377377
: params.finding.severity === 'medium'
378378
? 2
379379
: 3;
380-
// Insert a fresh manual-analysis claim, or revive a prior row that is in a
381-
// terminal state (completed/failed) so users can rerun analysis after a
382-
// previous attempt finished. Active rows (queued/pending/running) are left
383-
// alone and the caller treats this as a duplicate.
380+
// Insert a fresh manual-analysis claim, revive a terminal row, or let an
381+
// explicit user retry supersede an unclaimed scheduled retry. Pending and
382+
// running rows remain untouched because another launch already owns them.
384383
const rows = await db
385384
.insert(security_analysis_queue)
386385
.values({
@@ -410,10 +409,7 @@ export async function ensureManualAnalysisQueueRow(
410409
last_error_redacted: null,
411410
updated_at: sql`now()`,
412411
},
413-
setWhere: or(
414-
eq(security_analysis_queue.queue_status, 'completed'),
415-
eq(security_analysis_queue.queue_status, 'failed')
416-
),
412+
setWhere: inArray(security_analysis_queue.queue_status, ['queued', 'completed', 'failed']),
417413
})
418414
.returning({ id: security_analysis_queue.id });
419415
return rows.length > 0;

services/security-auto-analysis/src/manual-analysis.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -467,9 +467,9 @@ describe('ensureManualAnalysisQueueRow', () => {
467467
claim_token: 'claim-token',
468468
claimed_by_job_id: 'manual-job',
469469
});
470-
// Reviving terminal rows requires a setWhere clause that scopes the
471-
// ON CONFLICT update to completed/failed rows. Active rows (queued/
472-
// pending/running) must remain untouched and surface as duplicates.
470+
// Manual starts may revive terminal rows or supersede unclaimed queued
471+
// work. Active pending/running rows remain untouched and surface as
472+
// duplicates.
473473
expect(updateConfigs[0]).toMatchObject({
474474
set: expect.objectContaining({
475475
queue_status: 'pending',

services/security-auto-analysis/vitest.integration.config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ export default defineConfig({
44
test: {
55
globals: true,
66
environment: 'node',
7+
fileParallelism: false,
78
include: ['src/**/*.integration.test.ts'],
89
},
910
});

0 commit comments

Comments
 (0)