fix: audit stale is_latest rows cleared outside the batch

Signed-off-by: anilb <epipav@gmail.com>

Author: epipav

services/apps/packages_worker/src/packagist/__tests__/dueSelection.test.ts

@@ -13,6 +13,7 @@ import {
   markPackagistDailyProcessed,
   markPackagistMetadataScanned,
 } from '@crowd/data-access-layer/src/packages/packagistPackageState'
+import { upsertPackagistVersions } from '@crowd/data-access-layer/src/packages/versions'
 import type { QueryExecutor } from '@crowd/data-access-layer/src/queryExecutor'
 
 // Executes the real DAL functions against a capturing stand-in executor — no database.
@@ -256,3 +257,35 @@ describe('getCriticalPackagistPackageCount', () => {
     expect(sql).toMatch(/is_critical/)
   })
 })
+
+// The stale-is_latest cleanup only ever flips rows OUTSIDE the upsert batch (batch rows
+// already had is_latest set by the upsert CTE), so its row count must feed changedFields
+// separately or those real changes never reach the audit log.
+describe('upsertPackagistVersions is_latest cleanup audit', () => {
+  const versions = [
+    { number: '2.0.0', publishedAt: null, isLatest: true, isPrerelease: false, licenses: null },
+  ]
+
+  it("appends versions.is_latest when the cleanup cleared a stale row the CTE diff can't see", async () => {
+    qx.selectOne.mockResolvedValue({ changed_fields: [], version_ids: [] })
+    qx.result.mockResolvedValue(1)
+
+    const { changedFields } = await upsertPackagistVersions(asQx(qx), '7', versions, '2.0.0')
+
+    const [cleanupSql] = qx.result.mock.calls[0]
+    expect(cleanupSql).toMatch(/is_latest = false/)
+    expect(changedFields).toContain('versions.is_latest')
+  })
+
+  it('reports nothing extra when the cleanup cleared no rows, and never duplicates the CTE diff', async () => {
+    qx.selectOne.mockResolvedValue({ changed_fields: ['versions.is_latest'], version_ids: [] })
+    qx.result.mockResolvedValue(0)
+
+    const noClear = await upsertPackagistVersions(asQx(qx), '7', versions, '2.0.0')
+    expect(noClear.changedFields).toEqual(['versions.is_latest'])
+
+    qx.result.mockResolvedValue(2)
+    const cleared = await upsertPackagistVersions(asQx(qx), '7', versions, '2.0.0')
+    expect(cleared.changedFields.filter((f) => f === 'versions.is_latest')).toHaveLength(1)
+  })
+})

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

@@ -155,17 +155,24 @@ export async function upsertPackagistVersions(
   // records were updated differently can't keep is_latest = true alongside the new latest. When
   // no latest is known, leave flags untouched rather than wipe all. last_synced_at must move too
   // — it's the Tinybird ENGINE_VER, and this row's is_latest is a real change.
+  const changedFields = [...row.changed_fields]
   if (latestNumber != null) {
-    await qx.result(
+    const cleared = await qx.result(
       `UPDATE versions SET is_latest = false, last_synced_at = NOW()
          WHERE package_id = $(packageId)::bigint
            AND is_latest = true
            AND number <> $(latestNumber)`,
       { packageId, latestNumber },
     )
+    // The cleanup only ever flips rows OUTSIDE this batch (batch rows already got their
+    // is_latest set by the upsert above, so `is_latest = true` skips them) — the CTE diff
+    // can't see these changes, so surface them for the audit log here.
+    if (cleared > 0 && !changedFields.includes('versions.is_latest')) {
+      changedFields.push('versions.is_latest')
+    }
   }
 
-  return { changedFields: row.changed_fields, versionIds }
+  return { changedFields, versionIds }
 }
 
 export interface PypiVersionInput {