Skip to content

Commit 4e01e11

Browse files
committed
fix: reconcile OSV-dropped packages as range removals (CM-1258)
Signed-off-by: Uroš Marolt <uros@marolt.me>
1 parent e294bbc commit 4e01e11

2 files changed

Lines changed: 51 additions & 4 deletions

File tree

services/apps/packages_worker/src/osv/upsertAdvisory.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
AdvisoryRangeInsertInput,
33
findPackageId,
4+
findRemovedAdvisoryPackageIds,
45
reconcileOsvRanges,
56
supersedeDepsDevRanges,
67
upsertAdvisory,
@@ -33,11 +34,14 @@ export function dedupeRanges(ranges: NormalizedRange[]): NormalizedRange[] {
3334

3435
async function upsertOne(qx: QueryExecutor, record: NormalizedRecord): Promise<void> {
3536
const { advisory, packages } = record
36-
if (packages.length === 0) return
3737

3838
const advisoryId = await upsertAdvisory(qx, advisory)
3939

40-
const entries: { advisoryPackageId: number; ranges: AdvisoryRangeInsertInput[] }[] = []
40+
const entries: {
41+
advisoryPackageId: number
42+
ranges: AdvisoryRangeInsertInput[]
43+
supersedeDepsDev: boolean
44+
}[] = []
4145
for (const entry of packages) {
4246
const packageId = await findPackageId(qx, entry.pkg)
4347

@@ -56,9 +60,28 @@ async function upsertOne(qx: QueryExecutor, record: NormalizedRecord): Promise<v
5660
fixedVersion: range.fixedVersion,
5761
lastAffected: range.lastAffected,
5862
})),
63+
supersedeDepsDev: true,
5964
})
6065
}
6166

67+
// Packages OSV reported in a prior sync but that are missing from this payload
68+
// (parseOsvRecord drops a package once it has no usable ranges left, and a
69+
// corrected upstream record can drop one outright) never reach the loop above,
70+
// so their previously live OSV ranges would otherwise sit forever as false
71+
// positives and permanently block deps.dev's NOT EXISTS ownership guard.
72+
// Reconcile them too, with an empty range set — reconcileOsvRanges soft-deletes
73+
// every live OSV row when nothing in the new set matches. Not superseding
74+
// deps.dev here: OSV no longer covers the package, so a live deps.dev row (if
75+
// any) should stay live rather than be torn down alongside it.
76+
const removedIds = await findRemovedAdvisoryPackageIds(
77+
qx,
78+
advisoryId,
79+
entries.map((e) => e.advisoryPackageId),
80+
)
81+
for (const advisoryPackageId of removedIds) {
82+
entries.push({ advisoryPackageId, ranges: [], supersedeDepsDev: false })
83+
}
84+
6285
// Lock advisory_packages rows in ascending id order — the same order deps.dev's
6386
// bulk merge locks them in (ADVISORY_PACKAGES_LOCK_SQL's ORDER BY ap.id in
6487
// ingestAdvisories.ts). OSV's payload order is otherwise arbitrary; locking out
@@ -67,7 +90,7 @@ async function upsertOne(qx: QueryExecutor, record: NormalizedRecord): Promise<v
6790
// maximumAttempts: 1 turns a deadlock abort into a hard merge failure, not a retry.
6891
entries.sort((a, b) => a.advisoryPackageId - b.advisoryPackageId)
6992

70-
for (const { advisoryPackageId, ranges } of entries) {
93+
for (const { advisoryPackageId, ranges, supersedeDepsDev } of entries) {
7194
// Row-locks this advisory_packages row (held until the enclosing transaction
7295
// commits) before touching advisory_affected_ranges, matching the lock
7396
// deps.dev's bulk merge takes on the same row — whichever writer locks first
@@ -79,7 +102,7 @@ async function upsertOne(qx: QueryExecutor, record: NormalizedRecord): Promise<v
79102
})
80103

81104
await reconcileOsvRanges(qx, advisoryPackageId, ranges)
82-
await supersedeDepsDevRanges(qx, advisoryPackageId)
105+
if (supersedeDepsDev) await supersedeDepsDevRanges(qx, advisoryPackageId)
83106
}
84107
}
85108

services/libs/data-access-layer/src/packages/osv.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,30 @@ export async function upsertAdvisoryPackage(
139139
return row.id as number
140140
}
141141

142+
// advisory_packages rows OSV previously wrote for this advisory that are absent
143+
// from the current payload. parseOsvRecord drops a package once it has no
144+
// usable ranges left, and a corrected upstream record can drop a package
145+
// outright — either way the package never reaches upsertOne's per-entry loop,
146+
// so its previously live OSV ranges would sit as permanent false positives and
147+
// block deps.dev's NOT EXISTS ownership guard forever unless reconciled as a
148+
// removal (see upsertOne in services/apps/packages_worker/src/osv/upsertAdvisory.ts).
149+
export async function findRemovedAdvisoryPackageIds(
150+
qx: QueryExecutor,
151+
advisoryId: number,
152+
presentAdvisoryPackageIds: number[],
153+
): Promise<number[]> {
154+
const rows = await qx.select(
155+
`
156+
SELECT id
157+
FROM advisory_packages
158+
WHERE advisory_id = $(advisoryId)
159+
AND NOT (id = ANY($(presentAdvisoryPackageIds)::bigint[]))
160+
`,
161+
{ advisoryId, presentAdvisoryPackageIds },
162+
)
163+
return rows.map((r: { id: number }) => r.id)
164+
}
165+
142166
// Diff-based upsert + soft-delete for OSV-owned advisory_affected_ranges rows.
143167
// Replaces the old hard-delete + reinsert sweep (which minted a new PK for
144168
// every unchanged range every sync, producing zombie rows once Sequin

0 commit comments

Comments
 (0)