Skip to content

Commit ba7b467

Browse files
committed
fix: comments
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
1 parent 1839ac5 commit ba7b467

6 files changed

Lines changed: 63 additions & 7 deletions

File tree

backend/src/api/public/v1/packages/blastRadiusBatch.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ export const blastRadiusJobBatchRequestSchema = z.object({
2929
export type BlastRadiusJobBatchRequest = z.infer<typeof blastRadiusJobBatchRequestSchema>
3030

3131
// Unlike the read batches (purls that may or may not resolve to a package), every
32-
// job in a submit batch is genuinely submitted — there is no "not found" case, so
33-
// the response is a plain array in request order, not a found/not-found wrapper.
32+
// requested job produces a response entry — there is no "not found" case, so the
33+
// response is a plain array in request order, not a found/not-found wrapper.
3434
const analysisIdSchema = z.uuid()
3535

3636
export const blastRadiusJobPollBatchRequestSchema = z.object({

backend/src/api/public/v1/packages/getBlastRadiusJobBatch.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,47 @@ describe('getBlastRadiusJobBatch', () => {
137137
})
138138
})
139139

140+
it('matches an uppercase requestedAnalysisId against its (lowercase) row and echoes the original case', async () => {
141+
const uppercaseId = DONE_ID.toUpperCase()
142+
const { req, res, json } = mockReqRes({ analysisIds: [uppercaseId] })
143+
144+
getAnalysisDetailsByIds.mockResolvedValue([
145+
{
146+
id: DONE_ID,
147+
advisory_osv_id: 'GHSA-652q-gvq3-74qv',
148+
package_name: 'lodash',
149+
ecosystem: 'npm',
150+
status: 'done',
151+
error: null,
152+
candidates_considered: 10,
153+
started_at: '2026-07-01T00:00:00.000Z',
154+
completed_at: '2026-07-01T01:00:00.000Z',
155+
},
156+
])
157+
getVerdictResultsBatch.mockResolvedValue([
158+
{
159+
analysisId: DONE_ID,
160+
name: 'benchmark.js',
161+
version: '2.1.4',
162+
downloads: 500000,
163+
reachable_verdict: 'affected',
164+
confidence: 0.9,
165+
evidence: null,
166+
reasoning: 'uses merge',
167+
},
168+
])
169+
getDependentsExcludedByRangeCountBatch.mockResolvedValue([{ analysisId: DONE_ID, count: 8 }])
170+
171+
await getBlastRadiusJobBatch(req, res)
172+
173+
const [{ results }] = json.mock.calls[0]
174+
expect(results[0]).toMatchObject({ requestedAnalysisId: uppercaseId, found: true })
175+
expect(results[0].analysis).toMatchObject({
176+
status: 'done',
177+
summary: expect.objectContaining({ dependentsExcludedUpfront: 8 }),
178+
})
179+
})
180+
140181
it('rejects a batch with a malformed uuid without querying the database', async () => {
141182
const { req, res } = mockReqRes({ analysisIds: [PENDING_ID, 'not-a-uuid'] })
142183

backend/src/api/public/v1/packages/getBlastRadiusJobBatch.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ export async function getBlastRadiusJobBatch(req: Request, res: Response): Promi
2626
const qx = await getPackagesQx()
2727

2828
const analysisRows = await blastRadiusDal.getAnalysisDetailsByIds(qx, pagedAnalysisIds)
29-
const analysisById = new Map(analysisRows.map((row) => [row.id, row]))
29+
// Postgres normalizes uuid columns to lowercase on read, but a requested id can be
30+
// any case (schema only validates uuid shape) — normalize the lookup key so an
31+
// uppercase requestedAnalysisId still matches its (lowercase) row.
32+
const analysisById = new Map(analysisRows.map((row) => [row.id.toLowerCase(), row]))
3033

3134
const doneIds = analysisRows.filter((row) => row.status === 'done').map((row) => row.id)
3235
const [verdictRows, excludedByRangeCounts] = await Promise.all([
@@ -48,7 +51,7 @@ export async function getBlastRadiusJobBatch(req: Request, res: Response): Promi
4851
)
4952

5053
const results: BlastRadiusAnalysisBulkEntry[] = pagedAnalysisIds.map((requestedAnalysisId) => {
51-
const analysis = analysisById.get(requestedAnalysisId)
54+
const analysis = analysisById.get(requestedAnalysisId.toLowerCase())
5255
if (!analysis) {
5356
return { requestedAnalysisId, found: false, analysis: null }
5457
}
@@ -58,8 +61,8 @@ export async function getBlastRadiusJobBatch(req: Request, res: Response): Promi
5861
found: true,
5962
analysis: toBlastRadiusAnalysis(
6063
analysis,
61-
verdictsByAnalysisId.get(requestedAnalysisId) ?? [],
62-
excludedByRangeCountByAnalysisId.get(requestedAnalysisId) ?? 0,
64+
verdictsByAnalysisId.get(analysis.id) ?? [],
65+
excludedByRangeCountByAnalysisId.get(analysis.id) ?? 0,
6366
),
6467
}
6568
})

services/apps/packages_worker/src/bin/blast-radius-worker.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Config } from '@crowd/archetype-standard'
22
import { Options, ServiceWorker } from '@crowd/archetype-worker'
3+
import { SlackChannel } from '@crowd/slack'
34

45
// Own ServiceWorker instance rather than the shared one in ../service (used by every
56
// other packages_worker entry point) — its activities are CPU/IO-heavy (tarball
@@ -17,6 +18,7 @@ const config: Config = {
1718
const options: Options = {
1819
postgres: { enabled: false }, // packages-db is managed via getPackagesDb()
1920
maxConcurrentActivityTaskExecutions: 16,
21+
alertChannel: SlackChannel.CDP_AKRITES_ALERTS,
2022
}
2123

2224
const svc = new ServiceWorker(config, options)

services/apps/packages_worker/src/blast-radius/stages/dependents.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,14 @@ export async function runDependentsStage(
5555
signal,
5656
})
5757

58+
// scanDependents stops its loops early on abort but still returns whatever it
59+
// gathered so far — without this check, a cancelled/timed-out attempt would
60+
// persist that partial set and complete the stage successfully, and a Temporal
61+
// retry would then see it as already succeeded and skip re-scanning.
62+
if (signal?.aborted) {
63+
throw new Error('Dependents scan cancelled')
64+
}
65+
5866
// Resolve package_id for the analyzed set only (max topN=25) — these are the ones
5967
// actually surfaced in results/verdicts. excludedByRange (up to 200) never
6068
// reaches a result, so resolving it too would just be extra queries for nothing.

services/apps/packages_worker/src/blast-radius/workflows.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,11 @@ const { blastRadiusReachability } = proxyActivities<typeof activities>({
4141
retry: { maximumAttempts: 2 },
4242
})
4343

44+
// No heartbeatTimeout — runReportStage doesn't heartbeat until after it completes
45+
// (see activities.ts), so any run taking over a minute would otherwise always
46+
// heartbeat-timeout and retry before its first heartbeat.
4447
const { blastRadiusReport } = proxyActivities<typeof activities>({
4548
startToCloseTimeout: '2 minutes',
46-
heartbeatTimeout: '1 minute',
4749
retry: { maximumAttempts: 3 },
4850
})
4951

0 commit comments

Comments
 (0)