fix: audit all packagist changed-fields consistently

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

Author: epipav

docs/adr/0009-packagist-worker-design-decisions.md

@@ -1,7 +1,7 @@
-# ADR-0006: Packagist worker — design decisions (living)
+# ADR-0009: Packagist worker — design decisions (living)
 
 **Date**: 2026-07-13
-**Status**: living
+**Status**: accepted
 **Deciders**: Anil Bostanci
 
 _Living, consolidated record for the Packagist (PHP/Composer) worker. Record further
@@ -38,8 +38,8 @@ We store, in `package_dependencies`, **only the direct dependency edges a versio
 row per `(version_id, depends_on_id, dependency_kind)` — and we **resolve concrete versions and
 transitive closure at query time**, never at ingest.
 
-Per stored edge (for the critical slice only — dependency edges ride the same p2 metadata fetch as
-versions, which we only run for critical packages):
+Per stored edge (for **all** packages — dependency edges ride the same p2 metadata fetch as
+versions; see the Metadata enrichment scope decision):
 
 - `require` → `dependency_kind = 'direct'`, `require-dev` → `dependency_kind = 'dev'`.
 - `version_constraint` = the **declared** constraint string verbatim (e.g. `^3.0 || ^2.0`).
@@ -75,8 +75,7 @@ continuously re-running a Composer resolver for no durable benefit.
 **Why store `dev` edges when deps.dev doesn't:** deps.dev's resolved graph is runtime-only — every
 edge lands as `'direct'`, it has no dev/peer notion. Because we read the raw manifest we can split
 `require-dev` into `'dev'`, so Packagist edges are *richer per-edge* (dev deps + declared
-constraints) even though they are *shallower* (no resolved target version) and *narrower* (critical
-slice only) than deps.dev's.
+constraints) even though they are *shallower* (no resolved target version) than deps.dev's.
 
 **Consequences.**
 
@@ -98,8 +97,6 @@ _Negative / trade-offs:_
   way there is for npm/maven/pypi/cargo seeded from deps.dev.
 - Transitive questions ("everything X depends on") pay a recursive walk over direct edges at read
   time rather than a single indexed lookup.
-- Dependency edges exist for the **critical slice only** — non-critical Packagist packages have
-  none, because the p2 metadata crawl that produces them is critical-scoped.
 
 _Risks:_
 
@@ -198,3 +195,6 @@ that week's enrichment (recoverable via the manual trigger).
   covers repo linking for all packages and the deliberate critical-only carve-outs).
 - **2026-07-16** — Added _Lane architecture: seed → chained enrichment, downloads as dedicated
   lanes_ (removes the stats lane; renames `p2_last_modified` → `metadata_last_modified`).
+- **2026-07-17** — Corrected stale "critical slice only" wording left over in the _Dependency
+  model_ entry from before the _Metadata enrichment scope_ decision widened it to all packages;
+  set `Status` to `accepted` (matching ADR-0005's precedent for a living/consolidated doc).

docs/adr/README.md

@@ -15,7 +15,7 @@ Use the `/adr` skill in Claude Code to record new ADRs or query past decisions.
 | [ADR-0006](./0006-database-schema-types-as-source-of-truth.md) | Database schema types as the source of truth | accepted | 2026-07-09 |
 | [ADR-0007](./0007-test-factory-primitives-and-defaults.md) | Test factory primitives and defaults | accepted | 2026-07-10 |
 | [ADR-0008](./0008-how-we-write-unit-tests.md) | How we write unit tests | accepted | 2026-07-13 |
-| [ADR-0009](./0009-packagist-worker-design-decisions.md) | Packagist worker — design decisions (living) | living | 2026-07-13 |
+| [ADR-0009](./0009-packagist-worker-design-decisions.md) | Packagist worker — design decisions | accepted | 2026-07-13 |
 
 ## Why ADRs?
 

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

@@ -50,25 +50,30 @@ describe('monthlyWindowFor', () => {
 // packages.downloads_last_30d (npm parity). A window already recorded for the month
 // is never overwritten — the value is the observation closest to the boundary.
 describe('persistPackagist30dWindow', () => {
-  it('writes the window with the mirror when the month has no row yet', async () => {
-    await persistPackagist30dWindow(qx, PURL, 300, '2026-07-01')
+  it('writes the window with the mirror and returns the changed fields when the month has no row yet', async () => {
+    mockWindow.mockResolvedValue(['downloads_last_30d.count', 'packages.downloads_last_30d'])
+
+    const changedFields = await persistPackagist30dWindow(qx, PURL, 300, '2026-07-01')
 
     expect(mockExistingWindows).toHaveBeenCalledWith(qx, PURL, '2026-07-01', '2026-07-01')
     expect(mockWindow).toHaveBeenCalledWith(qx, PURL, '2026-06-01', '2026-07-01', 300, true)
+    expect(changedFields).toEqual(['downloads_last_30d.count', 'packages.downloads_last_30d'])
   })
 
-  it('does not overwrite an existing window for the month', async () => {
+  it('does not overwrite an existing window for the month, and returns no changed fields', async () => {
     mockExistingWindows.mockResolvedValue(['2026-07-01'])
 
-    await persistPackagist30dWindow(qx, PURL, 300, '2026-07-15')
+    const changedFields = await persistPackagist30dWindow(qx, PURL, 300, '2026-07-15')
 
     expect(mockWindow).not.toHaveBeenCalled()
+    expect(changedFields).toEqual([])
   })
 
-  it('writes nothing when the registry reported no monthly count', async () => {
-    await persistPackagist30dWindow(qx, PURL, null, '2026-07-01')
+  it('writes nothing and returns no changed fields when the registry reported no monthly count', async () => {
+    const changedFields = await persistPackagist30dWindow(qx, PURL, null, '2026-07-01')
 
     expect(mockExistingWindows).not.toHaveBeenCalled()
     expect(mockWindow).not.toHaveBeenCalled()
+    expect(changedFields).toEqual([])
   })
 })

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

@@ -110,7 +110,7 @@ describe('ingestOnePackagistMetadata', () => {
     })
   }
 
-  it('persists both endpoints, audits the merged fields, and marks scanned with the fresh Last-Modified', async () => {
+  it('audits phase 1 immediately and phase 2 separately, then marks scanned with the fresh Last-Modified', async () => {
     happyMocks()
 
     await ingestOnePackagistMetadata(qx, candidate)
@@ -119,12 +119,10 @@ describe('ingestOnePackagistMetadata', () => {
     expect(mockFetchP2).toHaveBeenCalledWith('monolog/monolog', 'Tue, 30 Jun 2026 00:00:00 GMT')
     expect(mockExpand).toHaveBeenCalledWith(minified)
     expect(mockPersistMetadata).toHaveBeenCalledWith(qx, PURL, expanded)
-    expect(mockAudit).toHaveBeenCalledWith(
-      qx,
-      'packagist',
-      PURL,
-      expect.arrayContaining(['packages.description', 'versions.number']),
-    )
+    // Two separate audit rows, not one merged call — phase 1 is logged as soon as it's
+    // committed, before the p2 fetch (which can throw) ever runs.
+    expect(mockAudit).toHaveBeenNthCalledWith(1, qx, 'packagist', PURL, ['packages.description'])
+    expect(mockAudit).toHaveBeenNthCalledWith(2, qx, 'packagist', PURL, ['versions.number'])
     expect(mockMarkMetadata).toHaveBeenCalledWith(
       qx,
       PURL,
@@ -227,24 +225,32 @@ describe('ingestOnePackagistMetadata', () => {
     )
   })
 
-  it('throws on a transient p2 result without marking scanned', async () => {
+  it('throws on a transient p2 result without marking scanned, but still audits phase 1', async () => {
     mockFetchStats.mockResolvedValue(statsJson as never)
-    mockPersistInfo.mockResolvedValue({ found: true, changedFields: [] })
+    mockPersistInfo.mockResolvedValue({ found: true, changedFields: ['packages.description'] })
     mockFetchP2.mockResolvedValue({ kind: 'TRANSIENT', message: 'HTTP 502' } as never)
 
     await expect(ingestOnePackagistMetadata(qx, candidate)).rejects.toThrow()
     expect(mockMarkMetadata).not.toHaveBeenCalled()
+    // phase-1 writes are already committed when the throw happens — a retry re-runs
+    // phase 1 idempotently and reports no changes, so this audit event can't be deferred
+    expect(mockAudit).toHaveBeenCalledWith(qx, 'packagist', PURL, ['packages.description'])
   })
 })
 
 // The monthly downloads-30d lane: dynamic fetch, window row only.
 describe('ingestOnePackagist30dWindow', () => {
-  it('persists the observed rolling window and marks the run processed', async () => {
+  it('persists the observed rolling window, audits the change, and marks the run processed', async () => {
     mockFetchStats.mockResolvedValue(statsJson as never)
+    mockPersist30d.mockResolvedValue(['downloads_last_30d.count', 'packages.downloads_last_30d'])
 
     await ingestOnePackagist30dWindow(qx, PURL, RUN_DATE)
 
     expect(mockPersist30d).toHaveBeenCalledWith(qx, PURL, 300, RUN_DATE)
+    expect(mockAudit).toHaveBeenCalledWith(qx, 'packagist', PURL, [
+      'downloads_last_30d.count',
+      'packages.downloads_last_30d',
+    ])
     expect(mockMark30d).toHaveBeenCalledWith(
       qx,
       PURL,
@@ -284,25 +290,31 @@ describe('ingestOnePackagist30dWindow', () => {
 describe('ingestOnePackagistDailyDownload', () => {
   const candidate = { purl: PURL, packageId: '7' }
 
-  it('inserts the daily row and marks the run processed', async () => {
+  it('inserts the daily row, audits the change, and marks the run processed', async () => {
     mockFetchStats.mockResolvedValue(statsJson as never)
+    mockDaily.mockResolvedValue(['downloads_daily.date', 'downloads_daily.count'])
 
     await ingestOnePackagistDailyDownload(qx, candidate, RUN_DATE)
 
     expect(mockDaily).toHaveBeenCalledWith(qx, '7', [{ day: RUN_DATE, downloads: 10 }])
+    expect(mockAudit).toHaveBeenCalledWith(qx, 'packagist', PURL, [
+      'downloads_daily.date',
+      'downloads_daily.count',
+    ])
     expect(mockMarkDaily).toHaveBeenCalledWith(
       qx,
       PURL,
       expect.objectContaining({ status: 'success' }),
     )
   })
 
-  it('marks success without inserting when the registry reports no daily count', async () => {
+  it('marks success without inserting or auditing when the registry reports no daily count', async () => {
     mockFetchStats.mockResolvedValue({ package: { name: 'monolog/monolog' } } as never)
 
     await ingestOnePackagistDailyDownload(qx, candidate, RUN_DATE)
 
     expect(mockDaily).not.toHaveBeenCalled()
+    expect(mockAudit).not.toHaveBeenCalled()
     expect(mockMarkDaily).toHaveBeenCalledWith(
       qx,
       PURL,

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

@@ -15,7 +15,7 @@ vi.mock('@crowd/data-access-layer/src/packages', () => ({
   updatePackagistVersionAggregates: vi.fn(),
   upsertPackagistVersions: vi.fn(),
   getPackagistPackageIdsByNames: vi.fn(),
-  upsertVersionDependencies: vi.fn().mockResolvedValue(0),
+  upsertVersionDependencies: vi.fn().mockResolvedValue([]),
 }))
 
 const mockAggregates = vi.mocked(updatePackagistVersionAggregates)
@@ -48,7 +48,7 @@ const expanded: PackagistExpandedVersion[] = [
 
 beforeEach(() => {
   vi.clearAllMocks()
-  mockDeps.mockResolvedValue(0)
+  mockDeps.mockResolvedValue([])
 })
 
 // C2 + C3 — persistence wiring: aggregates on packages, version rows, dependency edges
@@ -69,6 +69,7 @@ describe('persistPackagistMetadata', () => {
         ['phpunit/phpunit', '45'],
       ]),
     )
+    mockDeps.mockResolvedValue(['package_dependencies.depends_on_id'])
 
     const result = await persistPackagistMetadata(qx, PURL, expanded)
 
@@ -113,6 +114,8 @@ describe('persistPackagistMetadata', () => {
 
     expect(result.found).toBe(true)
     expect(result.unresolvedDependencyTargets).toBe(0)
+    // dependency-edge changes must reach the audit log alongside aggregates/versions
+    expect(result.changedFields).toContain('package_dependencies.depends_on_id')
   })
 
   it('skips and counts dependency targets that do not resolve to a packages row', async () => {

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

@@ -138,7 +138,8 @@ export async function ingestOnePackagistMetadata(
 
   const stats = normalizePackagistStats(info.value.package)
   const persistedInfo = await persistPackagistPackageInfo(qx, candidate.purl, stats)
-  const changedFields = [...persistedInfo.changedFields]
+  // Audit phase 1 immediately: those writes are already committed
+  await logAuditFieldChanges(qx, WORKER, candidate.purl, persistedInfo.changedFields)
 
   // Phase 2: p2 endpoint
   const p2 = await fetchWithFastRetry(
@@ -150,13 +151,12 @@ export async function ingestOnePackagistMetadata(
       { purl: candidate.purl, statusCode: p2.error.statusCode, kind: p2.error.kind },
       'packagist metadata 4xx/malformed after fast retries — marking scanned and skipping',
     )
-    // Phase-1 writes are already committed — their audit rows must not be dropped.
-    await logAuditFieldChanges(qx, WORKER, candidate.purl, changedFields)
     await markPackagistMetadataScanned(qx, candidate.purl, giveUpResult(p2.error))
     return
   }
 
   let lastModified: string | null = null
+  let phase2ChangedFields: string[] = []
   if (!isP2NotModified(p2.value)) {
     const expanded = expandComposerMetadata(p2.value.minifiedVersions)
     const persistResult = await persistPackagistMetadata(qx, candidate.purl, expanded)
@@ -166,11 +166,11 @@ export async function ingestOnePackagistMetadata(
         'packagist dependency targets not found in packages — edges skipped',
       )
     }
-    changedFields.push(...persistResult.changedFields)
+    phase2ChangedFields = persistResult.changedFields
     lastModified = p2.value.lastModified
   }
 
-  await logAuditFieldChanges(qx, WORKER, candidate.purl, changedFields)
+  await logAuditFieldChanges(qx, WORKER, candidate.purl, phase2ChangedFields)
   if (lastModified) {
     await markPackagistMetadataScanned(
       qx,
@@ -207,7 +207,13 @@ export async function ingestOnePackagist30dWindow(
     return
   }
 
-  await persistPackagist30dWindow(qx, purl, info.value.package.downloads?.monthly ?? null, runDate)
+  const changedFields = await persistPackagist30dWindow(
+    qx,
+    purl,
+    info.value.package.downloads?.monthly ?? null,
+    runDate,
+  )
+  await logAuditFieldChanges(qx, WORKER, purl, changedFields)
   await markPackagist30dProcessed(qx, purl, { status: 'success', attempts: info.attempts })
 }
 
@@ -234,7 +240,10 @@ export async function ingestOnePackagistDailyDownload(
 
   const daily = info.value.package.downloads?.daily
   if (typeof daily === 'number') {
-    await insertDailyDownloads(qx, candidate.packageId, [{ day: runDate, downloads: daily }])
+    const changedFields = await insertDailyDownloads(qx, candidate.packageId, [
+      { day: runDate, downloads: daily },
+    ])
+    await logAuditFieldChanges(qx, WORKER, candidate.purl, changedFields)
   }
   await markPackagistDailyProcessed(qx, candidate.purl, {
     status: 'success',

services/apps/packages_worker/src/packagist/activities.ts

@@ -21,18 +21,20 @@ export function monthlyWindowFor(runDate: string): { startDate: string; endDate:
 
 // One window row per purl per month, mirrored to packages.downloads_last_30d
 // (npm parity). A window already recorded for the month is never overwritten —
-// the value is the observation closest to the boundary.
+// the value is the observation closest to the boundary. Returns the changed
+// fields (like the sibling persist*/upsert* functions) — the caller audits them.
 export async function persistPackagist30dWindow(
   qx: QueryExecutor,
   purl: string,
   monthly: number | null,
   runDate: string,
-): Promise<void> {
-  if (monthly == null) return
+): Promise<string[]> {
+  if (monthly == null) return []
 
   const { startDate, endDate } = monthlyWindowFor(runDate)
   const existing = await getExistingLast30dEndDates(qx, purl, endDate, endDate)
   if (existing.length === 0) {
-    await upsertLast30dDownload(qx, purl, startDate, endDate, monthly, true)
+    return upsertLast30dDownload(qx, purl, startDate, endDate, monthly, true)
   }
+  return []
 }

services/apps/packages_worker/src/packagist/downloads.ts

@@ -88,7 +88,8 @@ export async function persistPackagistMetadata(
     }
 
     if (edges.length > 0) {
-      await upsertVersionDependencies(qx, edges)
+      const depChanges = await upsertVersionDependencies(qx, edges)
+      changedFields.push(...depChanges)
     }
   }
 

services/apps/packages_worker/src/packagist/upsertMetadata.ts

@@ -53,7 +53,8 @@ export async function persistPackagistPackageInfo(
 
   // Step 3: Maintainers only for critical packages
   if (isCritical && stats.maintainers.length > 0) {
-    await upsertPackageMaintainers(qx, id, stats.maintainers, 'packagist')
+    const maintainerChanges = await upsertPackageMaintainers(qx, id, stats.maintainers, 'packagist')
+    changedFields.push(...maintainerChanges)
   }
 
   return { found: true, changedFields }