fix: remove zombies

Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>

Author: ulemons

services/apps/packages_worker/src/maven/backfillRepositoryUrl.ts

@@ -1,11 +1,13 @@
 import {
+  deleteMavenPackageRepoLinks,
   listMavenPackagesForRepoUrlRecompute,
   updateMavenRepositoryUrls,
 } from '@crowd/data-access-layer'
 import { QueryExecutor } from '@crowd/data-access-layer/src/queryExecutor'
 import { getServiceChildLogger } from '@crowd/logging'
 
 import { normalizeScmUrl } from './extract'
+import { writeRepoLink } from './runMavenEnrichmentLoop'
 
 const log = getServiceChildLogger('maven-repo-url-backfill')
 
@@ -15,6 +17,8 @@ export type RepoUrlBackfillTotals = {
   cleared: number // Gap C: non-repo value → NULL
   rewritten: number // non-canonical value → different canonical value
   unchanged: number
+  linked: number // repos/package_repos link (re)written for a fill or rewrite
+  pruned: number // stale 'declared' link removed for a clear or rewrite
 }
 
 /**
@@ -23,6 +27,15 @@ export type RepoUrlBackfillTotals = {
  * fetched — the raw SCM value is already in the DB. Fills recoverable NULLs
  * (Gap B) and clears non-repository values (Gap C) via direct UPDATE.
  *
+ * Link tables: package_repos is kept consistent with the recomputed
+ * repository_url. For rows that had a value and now change (rewrites) or are
+ * cleared, the stale source='declared' link is deleted; for rows that gain a
+ * canonical URL (fills and rewrites) the correct link is (re)written via the
+ * same `writeRepoLink` the enrichment loop uses. This matters because consumers
+ * such as security-contacts read the repo through repos ⋈ package_repos, not
+ * packages.repository_url. Note this is stricter than the incremental
+ * enrichment path, which is upsert-only and never prunes.
+ *
  * Idempotent and resumable: the id cursor is derived from the scan, so a
  * re-run after an interrupt simply reprocesses from the start and skips rows
  * that already match.
@@ -38,6 +51,8 @@ export async function backfillMavenRepositoryUrls(
     cleared: 0,
     rewritten: 0,
     unchanged: 0,
+    linked: 0,
+    pruned: 0,
   }
 
   let afterId = 0
@@ -51,6 +66,10 @@ export async function backfillMavenRepositoryUrls(
     if (rows.length === 0) break
 
     const updates: { id: number; repositoryUrl: string | null }[] = []
+    // Rows that had a link and now change/clear — their stale 'declared' link is pruned.
+    const pruneTargets: number[] = []
+    // Rows that gained a canonical URL — their repo link is (re)written after the update.
+    const linkTargets: { id: number; repositoryUrl: string }[] = []
     for (const row of rows) {
       totals.scanned++
       const desired = normalizeScmUrl(row.declaredRepositoryUrl)
@@ -62,10 +81,27 @@ export async function backfillMavenRepositoryUrls(
       else if (desired === null) totals.cleared++
       else totals.rewritten++
       updates.push({ id: row.id, repositoryUrl: desired })
+      // A 'declared' link only exists when the row already had a value.
+      if (row.repositoryUrl !== null) pruneTargets.push(row.id)
+      if (desired !== null) linkTargets.push({ id: row.id, repositoryUrl: desired })
     }
 
     if (updates.length > 0 && !dryRun) {
-      await updateMavenRepositoryUrls(qx, updates)
+      // Atomic per batch: the repository_url UPDATE, the stale-link prune, and the
+      // relink must commit together. Otherwise an interrupt between them leaves
+      // packages.repository_url updated but package_repos out of sync — and on a
+      // re-run the row is skipped (its repository_url already matches `desired`),
+      // so the inconsistency would never be repaired. On rollback the row stays
+      // unchanged and is reprocessed on the next run.
+      await qx.tx(async (t) => {
+        await updateMavenRepositoryUrls(t, updates)
+        await deleteMavenPackageRepoLinks(t, pruneTargets)
+        for (const target of linkTargets) {
+          await writeRepoLink(t, target.id, target.repositoryUrl)
+        }
+      })
+      totals.pruned += pruneTargets.length
+      totals.linked += linkTargets.length
     }
 
     afterId = rows[rows.length - 1].id

services/apps/packages_worker/src/maven/runMavenEnrichmentLoop.ts

@@ -53,7 +53,7 @@ type PackageRow = MavenPackageToSync
 // ─── Helpers ──────────────────────────────────────────────────────────────────
 
 // prettier-ignore
-async function writeRepoLink(qx: QueryExecutor, packageId: number, repositoryUrl: string | null, changed: Set<string>): Promise<void> {
+export async function writeRepoLink(qx: QueryExecutor, packageId: number, repositoryUrl: string | null, changed?: Set<string>): Promise<void> {
   if (!repositoryUrl) return
   const parsed = parseRepoUrl(repositoryUrl)
   if (!parsed) return
@@ -64,7 +64,7 @@ async function writeRepoLink(qx: QueryExecutor, packageId: number, repositoryUrl
     source: 'declared',
     confidence: 0.8,
   })
-  repoChanged.forEach((f) => changed.add(f))
+  repoChanged.forEach((f) => changed?.add(f))
 }
 
 // Postgres deadlock (40P01) is transient: concurrent transactions upserting the same shared

services/libs/data-access-layer/src/osspckgs/repos.ts

@@ -36,6 +36,26 @@ export async function upsertRepo(qx: QueryExecutor, item: IDbRepoUpsert): Promis
   return row.id as number
 }
 
+/**
+ * Removes the `declared` repo links for the given packages. Used by the
+ * repository_url backfill to drop stale links when a package's canonical URL
+ * changes or is cleared, keeping package_repos consistent with
+ * packages.repository_url. Only touches source='declared' (the link Maven
+ * owns) — links from other enrichers (deps.dev, heuristic, manual) are left
+ * untouched. The shared `repos` rows are never deleted.
+ */
+export async function deleteMavenPackageRepoLinks(
+  qx: QueryExecutor,
+  packageIds: number[],
+): Promise<void> {
+  if (packageIds.length === 0) return
+  await qx.result(
+    `DELETE FROM package_repos
+     WHERE package_id = ANY($(packageIds)::bigint[]) AND source = 'declared'`,
+    { packageIds },
+  )
+}
+
 /**
  * Links a package to a repo with provenance metadata.
  * On conflict keeps the higher confidence value and refreshes verified_at.