fix(lib-node): version-key extraction path to bust ESM module cache

lib-node/index.ts

@@ -1,7 +1,7 @@
 import { createGunzip } from 'node:zlib'
 import { pipeline } from 'node:stream/promises'
 import { createWriteStream } from 'node:fs'
-import { mkdir, readFile, writeFile, rm, rename, stat, utimes } from 'node:fs/promises'
+import { mkdir, readFile, writeFile, rm, rename, stat, utimes, readdir } from 'node:fs/promises'
 import { join, dirname } from 'node:path'
 import { spawn } from 'node:child_process'
 import * as tar from 'tar-stream'
@@ -53,10 +53,54 @@ export interface EnsureArtefactResult {
   downloaded: boolean
 }
 
-interface CacheMeta {
+// Stable per-buildTuple pointer file. It survives across versions and drives
+// the conditional GET; its `dir` points at the version-keyed extraction dir
+// (relative to artefactDir) currently in use for this buildTuple.
+interface CachePointer {
   dataUpdatedAt: string
   version: string
-  hasNativeModules: boolean
+  /** `<version>+<epochSeconds>/<buildTuple>`, relative to the artefact dir. */
+  dir: string
+}
+
+// Filesystem-safe discriminator: dataUpdatedAt as an epoch-SECONDS integer.
+// Second precision is deliberate — it matches the conditional-GET criterion
+// exactly: both the registry's If-Modified-Since check and the Last-Modified
+// header it derives from work at `Math.floor(getTime() / 1000)`. Keying the
+// path on the same granularity guarantees two dataUpdatedAt values the server
+// treats as identical (→ 304) map to the same dir, and any value it treats as
+// changed (→ 200) maps to a new one. (The raw ISO string can't be used
+// directly: colons are invalid path chars on some filesystems.)
+const discriminator = (dataUpdatedAt: string): string =>
+  String(Math.floor(new Date(dataUpdatedAt).getTime() / 1000))
+
+// Top-level version-dir segment (`<version>+<epochSeconds>`) of a relative pointer dir.
+const versionDirOf = (relDir: string): string => relDir.split(/[\\/]/)[0]
+
+// Remove version dirs no longer referenced by any buildTuple pointer. Lazy and
+// best-effort: anything already imported lives in memory, so dropping its
+// on-disk dir is safe; an old dir held by a concurrent in-flight run is the
+// only edge, hence we swallow errors rather than fail the call.
+const pruneOldVersionDirs = async (artefactDir: string): Promise<void> => {
+  let entries
+  try {
+    entries = await readdir(artefactDir, { withFileTypes: true })
+  } catch {
+    return
+  }
+  const keep = new Set<string>()
+  for (const e of entries) {
+    if (e.isFile() && e.name.startsWith('.pointer-') && e.name.endsWith('.json')) {
+      try {
+        const p = JSON.parse(await readFile(join(artefactDir, e.name), 'utf-8')) as Partial<CachePointer>
+        if (typeof p.dir === 'string') keep.add(versionDirOf(p.dir))
+      } catch { /* ignore unreadable pointer */ }
+    }
+  }
+  for (const e of entries) {
+    if (!e.isDirectory() || keep.has(e.name)) continue
+    await rm(join(artefactDir, e.name), { recursive: true, force: true }).catch(() => {})
+  }
 }
 
 export async function ensureArtefact (opts: EnsureArtefactOpts): Promise<EnsureArtefactResult> {
@@ -67,26 +111,25 @@ export async function ensureArtefact (opts: EnsureArtefactOpts): Promise<EnsureA
   const encodedId = encodeURIComponent(opts.artefactId)
   const artefactDir = join(opts.cacheDir, opts.artefactId)
   const buildTuple = opts.build ? `${nodeMajor()}-${detectLibc()}` : 'js'
-  const extractDir = join(artefactDir, buildTuple)
-  const metaPath = join(extractDir, '.meta.json')
+  const pointerPath = join(artefactDir, `.pointer-${buildTuple}.json`)
 
-  // Read existing cache meta (if any) to drive the conditional GET. An older
-  // cache missing the new fields is treated as a cold cache — the server
-  // returns 200, we re-extract once, and the new meta supersedes it.
-  let cachedMeta: CacheMeta | null = null
+  // Read the existing pointer (if any) to drive the conditional GET. A missing
+  // or legacy cache is treated as cold — the server returns 200, we extract to
+  // a fresh version dir, and the new pointer supersedes whatever was there.
+  let pointer: CachePointer | null = null
   try {
-    const raw = await readFile(metaPath, 'utf-8')
-    const parsed = JSON.parse(raw) as Partial<CacheMeta>
-    if (parsed.dataUpdatedAt && parsed.version && typeof parsed.hasNativeModules === 'boolean') {
-      cachedMeta = parsed as CacheMeta
+    const raw = await readFile(pointerPath, 'utf-8')
+    const parsed = JSON.parse(raw) as Partial<CachePointer>
+    if (parsed.dataUpdatedAt && parsed.version !== undefined && parsed.dir) {
+      pointer = parsed as CachePointer
     }
   } catch {
-    // cold cache or invalid metadata
+    // cold cache or invalid/legacy metadata
   }
 
   const reqHeaders: Record<string, string> = {}
-  if (cachedMeta) {
-    reqHeaders['if-modified-since'] = new Date(cachedMeta.dataUpdatedAt).toUTCString()
+  if (pointer) {
+    reqHeaders['if-modified-since'] = new Date(pointer.dataUpdatedAt).toUTCString()
   }
 
   const res = await ax.get(`/api/v1/artefacts/${encodedId}/download`, {
@@ -98,9 +141,9 @@ export async function ensureArtefact (opts: EnsureArtefactOpts): Promise<EnsureA
   if (res.status === 304) {
     ;(res.data as Readable).destroy()
     return {
-      path: extractDir,
-      version: cachedMeta!.version,
-      dataUpdatedAt: cachedMeta!.dataUpdatedAt,
+      path: join(artefactDir, pointer!.dir),
+      version: pointer!.version,
+      dataUpdatedAt: pointer!.dataUpdatedAt,
       downloaded: false
     }
   }
@@ -113,14 +156,19 @@ export async function ensureArtefact (opts: EnsureArtefactOpts): Promise<EnsureA
     ? new Date(lastModified).toISOString()
     : new Date().toISOString()
 
+  // Extract into a content-versioned dir so a changed artefact resolves to a
+  // brand-new absolute path — forcing Node's ESM module registry to reload the
+  // entire graph (entry + siblings + bundled deps), not just the query-busted
+  // entry. `version` is kept in the name for readability only; the seconds
+  // suffix is what guarantees uniqueness (version can repeat or be empty).
+  const relDir = join(`${version || '0.0.0'}+${discriminator(dataUpdatedAt)}`, buildTuple)
+  const extractDir = join(artefactDir, relDir)
+
   const tmpDir = `${extractDir}.tmp.${process.pid}`
   await rm(tmpDir, { recursive: true, force: true })
   await mkdir(tmpDir, { recursive: true })
   try {
     await extractTarball(res.data as Readable, tmpDir)
-    // Write meta inside tmpDir so it survives the rename atomically.
-    const meta: CacheMeta = { dataUpdatedAt, version, hasNativeModules }
-    await writeFile(join(tmpDir, '.meta.json'), JSON.stringify(meta))
     if (opts.build && hasNativeModules) {
       await rebuildNativeModules(tmpDir)
     }
@@ -131,6 +179,13 @@ export async function ensureArtefact (opts: EnsureArtefactOpts): Promise<EnsureA
   await rm(extractDir, { recursive: true, force: true })
   await rename(tmpDir, extractDir)
 
+  // Atomically repoint the stable pointer at the new dir, then prune the dirs
+  // no pointer references anymore.
+  const tmpPointer = `${pointerPath}.tmp.${process.pid}`
+  await writeFile(tmpPointer, JSON.stringify({ dataUpdatedAt, version, dir: relDir } satisfies CachePointer))
+  await rename(tmpPointer, pointerPath)
+  await pruneOldVersionDirs(artefactDir)
+
   return { path: extractDir, version, dataUpdatedAt, downloaded: true }
 }
 

lib-node/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@data-fair/lib-node-registry",
-  "version": "0.6.0",
+  "version": "0.7.0",
   "description": "Node.js client library for the data-fair registry service.",
   "type": "module",
   "license": "MIT",

tests/lib-node-cache.unit.spec.ts

@@ -0,0 +1,221 @@
+import { test, expect } from '@playwright/test'
+import { join } from 'node:path'
+import { readFile, rm, mkdir, readdir, access } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import http from 'node:http'
+import { AddressInfo } from 'node:net'
+import { createTestTarball } from './support/test-tarball.ts'
+import { ensureArtefact } from '../lib-node/index.ts'
+
+// A self-contained fake registry exposing only GET /:id/download, mirroring the
+// real endpoint's headers and conditional-GET semantics. Tests mutate `state`
+// between calls to simulate (re-)uploads without needing the full dev stack.
+interface FakeState {
+  tarball: Buffer
+  version: string
+  lastModified: Date
+  hasNativeModules: boolean
+}
+
+const startFakeRegistry = async (state: FakeState) => {
+  const server = http.createServer((req, res) => {
+    if (!req.url?.includes('/download')) {
+      res.statusCode = 404
+      res.end()
+      return
+    }
+    res.setHeader('Last-Modified', state.lastModified.toUTCString())
+    res.setHeader('Cache-Control', 'no-cache')
+    res.setHeader('X-Artefact-Version', state.version)
+    res.setHeader('X-Artefact-Has-Native-Modules', state.hasNativeModules ? 'true' : 'false')
+
+    const ims = req.headers['if-modified-since']
+    if (ims) {
+      const since = Math.floor(new Date(ims).getTime() / 1000)
+      const mod = Math.floor(state.lastModified.getTime() / 1000)
+      if (!isNaN(since) && since >= mod) {
+        res.statusCode = 304
+        res.end()
+        return
+      }
+    }
+    res.statusCode = 200
+    res.setHeader('Content-Type', 'application/gzip')
+    res.end(state.tarball)
+  })
+  await new Promise<void>(resolve => server.listen(0, '127.0.0.1', resolve))
+  const port = (server.address() as AddressInfo).port
+  return {
+    url: `http://127.0.0.1:${port}`,
+    close: () => new Promise<void>(resolve => server.close(() => resolve()))
+  }
+}
+
+test.describe('lib-node version-keyed extraction', () => {
+  let cacheDir: string
+  const artefactId = '@test/pkg@1'
+  const secretKey = 'secret-internal'
+
+  test.beforeEach(async () => {
+    cacheDir = join(tmpdir(), `registry-cache-unit-${process.pid}-${Math.floor(performance.now())}`)
+    await mkdir(cacheDir, { recursive: true })
+  })
+
+  test.afterEach(async () => {
+    await rm(cacheDir, { recursive: true, force: true })
+  })
+
+  test('extraction path changes when artefact content changes', async () => {
+    const state: FakeState = {
+      tarball: await createTestTarball({ name: '@test/pkg', version: '1.0.0' }),
+      version: '1.0.0',
+      lastModified: new Date('2026-01-01T00:00:00Z'),
+      hasNativeModules: false
+    }
+    const registry = await startFakeRegistry(state)
+    try {
+      const opts = { registryUrl: registry.url, secretKey, artefactId, cacheDir }
+      const r1 = await ensureArtefact(opts)
+      expect(r1.downloaded).toBe(true)
+
+      // Simulate a re-upload: new content + newer Last-Modified.
+      state.tarball = await createTestTarball({ name: '@test/pkg', version: '1.0.1' })
+      state.version = '1.0.1'
+      state.lastModified = new Date('2026-01-02T00:00:00Z')
+
+      const r2 = await ensureArtefact(opts)
+      expect(r2.downloaded).toBe(true)
+      expect(r2.version).toBe('1.0.1')
+      // The core fix: a content change yields a brand-new extraction path so
+      // Node's ESM module registry (keyed by resolved URL) reloads the graph.
+      expect(r2.path).not.toBe(r1.path)
+    } finally {
+      await registry.close()
+    }
+  })
+
+  test('same-version re-publish (dataUpdatedAt moves) also changes the path', async () => {
+    const state: FakeState = {
+      tarball: await createTestTarball({ name: '@test/pkg', version: '1.0.0' }),
+      version: '1.0.0',
+      lastModified: new Date('2026-01-01T00:00:00Z'),
+      hasNativeModules: false
+    }
+    const registry = await startFakeRegistry(state)
+    try {
+      const opts = { registryUrl: registry.url, secretKey, artefactId, cacheDir }
+      const r1 = await ensureArtefact(opts)
+
+      // Same version string, but a fresh upload bumps dataUpdatedAt.
+      state.lastModified = new Date('2026-01-01T00:00:05Z')
+      const r2 = await ensureArtefact(opts)
+      expect(r2.downloaded).toBe(true)
+      expect(r2.version).toBe('1.0.0')
+      expect(r2.path).not.toBe(r1.path)
+    } finally {
+      await registry.close()
+    }
+  })
+
+  test('returns the same extraction path on an unchanged (304) call', async () => {
+    const state: FakeState = {
+      tarball: await createTestTarball({ name: '@test/pkg', version: '1.0.0' }),
+      version: '1.0.0',
+      lastModified: new Date('2026-01-01T00:00:00Z'),
+      hasNativeModules: false
+    }
+    const registry = await startFakeRegistry(state)
+    try {
+      const opts = { registryUrl: registry.url, secretKey, artefactId, cacheDir }
+      const r1 = await ensureArtefact(opts)
+      expect(r1.downloaded).toBe(true)
+      const r2 = await ensureArtefact(opts)
+      expect(r2.downloaded).toBe(false)
+      expect(r2.path).toBe(r1.path)
+      // Content is still readable from the cached path.
+      const pkg = JSON.parse(await readFile(join(r2.path, 'package.json'), 'utf-8'))
+      expect(pkg.name).toBe('@test/pkg')
+    } finally {
+      await registry.close()
+    }
+  })
+
+  test('writes a stable per-buildTuple pointer file', async () => {
+    const state: FakeState = {
+      tarball: await createTestTarball({ name: '@test/pkg', version: '1.0.0' }),
+      version: '1.0.0',
+      lastModified: new Date('2026-01-01T00:00:00Z'),
+      hasNativeModules: false
+    }
+    const registry = await startFakeRegistry(state)
+    try {
+      const opts = { registryUrl: registry.url, secretKey, artefactId, cacheDir }
+      const r1 = await ensureArtefact(opts)
+
+      const artefactDir = join(cacheDir, artefactId)
+      const pointer = JSON.parse(await readFile(join(artefactDir, '.pointer-js.json'), 'utf-8'))
+      expect(pointer.dataUpdatedAt).toBe(r1.dataUpdatedAt)
+      expect(pointer.version).toBe('1.0.0')
+      // The pointer's dir resolves to the returned extraction path.
+      expect(join(artefactDir, pointer.dir)).toBe(r1.path)
+    } finally {
+      await registry.close()
+    }
+  })
+
+  test('prunes the previous version directory after a re-download', async () => {
+    const state: FakeState = {
+      tarball: await createTestTarball({ name: '@test/pkg', version: '1.0.0' }),
+      version: '1.0.0',
+      lastModified: new Date('2026-01-01T00:00:00Z'),
+      hasNativeModules: false
+    }
+    const registry = await startFakeRegistry(state)
+    try {
+      const opts = { registryUrl: registry.url, secretKey, artefactId, cacheDir }
+      const r1 = await ensureArtefact(opts)
+
+      state.tarball = await createTestTarball({ name: '@test/pkg', version: '1.0.1' })
+      state.version = '1.0.1'
+      state.lastModified = new Date('2026-01-02T00:00:00Z')
+      const r2 = await ensureArtefact(opts)
+
+      // The old extraction directory is gone; only the current one remains.
+      await expect(access(r1.path)).rejects.toBeTruthy()
+      await expect(access(r2.path)).resolves.toBeUndefined()
+
+      // No stray versioned dirs accumulate (just the pointer file + one dir).
+      const artefactDir = join(cacheDir, artefactId)
+      const entries = (await readdir(artefactDir)).filter(e => !e.startsWith('.'))
+      expect(entries).toHaveLength(1)
+    } finally {
+      await registry.close()
+    }
+  })
+
+  test('pruning keeps a version dir still referenced by another buildTuple', async () => {
+    const state: FakeState = {
+      tarball: await createTestTarball({ name: '@test/pkg', version: '1.0.0' }),
+      version: '1.0.0',
+      lastModified: new Date('2026-01-01T00:00:00Z'),
+      hasNativeModules: false
+    }
+    const registry = await startFakeRegistry(state)
+    try {
+      const base = { registryUrl: registry.url, secretKey, artefactId, cacheDir }
+      // The "js" (build:false) consumer warms its slot against the v1 content.
+      const jsResult = await ensureArtefact(base)
+
+      // Content changes, then a build:true consumer downloads the new version.
+      // Its prune pass must not delete the js slot's still-current version dir.
+      state.tarball = await createTestTarball({ name: '@test/pkg', version: '1.0.1' })
+      state.version = '1.0.1'
+      state.lastModified = new Date('2026-01-02T00:00:00Z')
+      await ensureArtefact({ ...base, build: true })
+
+      await expect(access(jsResult.path)).resolves.toBeUndefined()
+    } finally {
+      await registry.close()
+    }
+  })
+})