fix(core): refresh stale plugin cache

packages/core/src/npm.ts

@@ -2,6 +2,7 @@ export * as Npm from "./npm"
 
 import path from "path"
 import npa from "npm-package-arg"
+import semver from "semver"
 import { Effect, Schema, Context, Layer, Option, FileSystem } from "effect"
 import { NodeFileSystem } from "@effect/platform-node"
 import { FSUtil } from "./fs-util"
@@ -24,8 +25,14 @@ export interface EntryPoint {
   readonly entrypoint?: string
 }
 
+export interface AddOptions {
+  // Force a re-resolve against the registry even for pinned/file specs,
+  // bypassing the `dist-tags` freshness check.
+  readonly refresh?: boolean
+}
+
 export interface Interface {
-  readonly add: (pkg: string) => Effect.Effect<EntryPoint, InstallFailedError | EffectFlock.LockError>
+  readonly add: (pkg: string, options?: AddOptions) => Effect.Effect<EntryPoint, InstallFailedError | EffectFlock.LockError>
   readonly install: (
     dir: string,
     input?: {
@@ -69,6 +76,33 @@ interface ArboristTree {
   edgesOut: Map<string, { to?: ArboristNode }>
 }
 
+// Floating registry specs (`@latest`, ranges, bare names) re-resolve against
+// the registry on each load. Pinned versions, file/git/remote specs, and
+// aliases are immutable cache keys; aliases delegate to a subSpec that is
+// pinned or resolvable at install time, so the alias name itself is not a
+// registry target.
+export function isMutableRegistrySpec(spec: string) {
+  try {
+    const hit = npa(spec)
+    if (!hit.name) return false
+    if (hit.type === "git" || hit.type === "remote" || hit.type === "directory" || hit.type === "file") return false
+    if (hit.type === "version" || hit.type === "alias") return false
+    if (hit.type === "tag" && hit.fetchSpec && hit.fetchSpec !== "latest" && hit.fetchSpec !== "*") return false
+    return true
+  } catch {
+    return false
+  }
+}
+
+export function shouldRefreshInstall(installed: string | undefined, registry: string | undefined) {
+  if (!installed || !registry) return false
+  const installedVersion = semver.valid(installed)
+  const registryVersion = semver.valid(registry)
+  // Non-semver values (e.g. dist-tag strings) fall back to string inequality.
+  if (!installedVersion || !registryVersion) return installed !== registry
+  return semver.gt(registryVersion, installedVersion)
+}
+
 const layer = Layer.effect(
   Service,
   Effect.gen(function* () {
@@ -112,7 +146,28 @@ const layer = Layer.effect(
         }),
       )
 
-    const add = Effect.fn("Npm.add")(function* (pkg: string) {
+    // Resolves only the `latest` dist-tag, a heuristic that misses newer
+    // satisfying versions for range specs and sends no auth headers (private
+    // registries may 401). Any failure collapses to `undefined` so the cached
+    // install is kept.
+    const fetchRegistryVersion = (name: string, cacheDir: string) =>
+      Effect.gen(function* () {
+        const registry = yield* NpmConfig.registry(cacheDir)
+        const encoded = name.startsWith("@") ? name.replace("/", "%2F") : name
+        const response = yield* Effect.tryPromise(() =>
+          fetch(`${registry}/${encoded}`, {
+            headers: { Accept: "application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8" },
+            signal: AbortSignal.timeout(5_000),
+          }),
+        ).pipe(Effect.orElseSucceed(() => undefined))
+        if (!response?.ok) return undefined
+        const data = yield* Effect.tryPromise(() =>
+          response.json() as Promise<{ "dist-tags"?: Record<string, string> }>,
+        ).pipe(Effect.orElseSucceed(() => undefined))
+        return data?.["dist-tags"]?.latest
+      })
+
+    const add = Effect.fn("Npm.add")(function* (pkg: string, options?: AddOptions) {
       const dir = directory(pkg)
       const name = (() => {
         try {
@@ -121,15 +176,40 @@ const layer = Layer.effect(
           return pkg
         }
       })()
-
-      if (yield* afs.existsSafe(path.join(dir, "node_modules", name))) {
-        return resolveEntryPoint(name, path.join(dir, "node_modules", name))
+      const modulePath = path.join(dir, "node_modules", name)
+
+      if (yield* afs.existsSafe(modulePath)) {
+        const refresh =
+          options?.refresh ||
+          (isMutableRegistrySpec(pkg) &&
+            shouldRefreshInstall(
+              yield* afs.readJson(path.join(modulePath, "package.json")).pipe(
+                Effect.map((json) => (json as { version?: string }).version),
+                Effect.catch(() => Effect.succeed(undefined)),
+              ),
+              yield* fetchRegistryVersion(name, dir),
+            ))
+        if (!refresh) return resolveEntryPoint(name, modulePath)
+        // Drop the lockfile so `reify` re-resolves the floating dist-tag. If
+        // `reify` fails (transient registry/network error, or lock
+        // contention), fall back to the cached install instead of breaking
+        // the plugin load. The catch is intentionally broad: a stale-but-
+        // working plugin beats a dead one, matching the silent degradation
+        // used for `fetchRegistryVersion` failures above.
+        yield* fs.remove(path.join(dir, "package-lock.json")).pipe(Effect.ignore)
+        return yield* reify({ dir, add: [pkg] }).pipe(
+          Effect.map((tree) => {
+            const first = tree.edgesOut.values().next().value?.to
+            return first ? resolveEntryPoint(first.name, first.path) : resolveEntryPoint(name, modulePath)
+          }),
+          Effect.catch(() => Effect.succeed(resolveEntryPoint(name, modulePath))),
+        )
       }
 
       const tree = yield* reify({ dir, add: [pkg] })
       const first = tree.edgesOut.values().next().value?.to
       if (!first) {
-        const result = resolveEntryPoint(name, path.join(dir, "node_modules", name))
+        const result = resolveEntryPoint(name, modulePath)
         if (result.entrypoint) return result
         return yield* new InstallFailedError({ add: [pkg], dir })
       }

packages/core/test/npm.test.ts

@@ -35,7 +35,51 @@ describe("Npm.sanitize", () => {
   })
 })
 
+describe("Npm.isMutableRegistrySpec", () => {
+  test("treats latest and floating registry specs as mutable", () => {
+    expect(Npm.isMutableRegistrySpec("acme@latest")).toBe(true)
+    expect(Npm.isMutableRegistrySpec("acme@^1.0.0")).toBe(true)
+    expect(Npm.isMutableRegistrySpec("@opencode/acme@latest")).toBe(true)
+    expect(Npm.isMutableRegistrySpec("acme")).toBe(true)
+  })
+
+  test("treats pinned versions and non-registry specs as immutable", () => {
+    expect(Npm.isMutableRegistrySpec("acme@1.2.3")).toBe(false)
+    expect(Npm.isMutableRegistrySpec("acme@beta")).toBe(false)
+    expect(Npm.isMutableRegistrySpec("acme@git+https://github.com/opencode/acme.git")).toBe(false)
+    expect(Npm.isMutableRegistrySpec(`acme@file:${path.join("/tmp", "acme")}`)).toBe(false)
+    expect(Npm.isMutableRegistrySpec("acme@npm:other@1.0.0")).toBe(false)
+    expect(Npm.isMutableRegistrySpec("not a valid spec!!!")).toBe(false)
+  })
+})
+
+describe("Npm.shouldRefreshInstall", () => {
+  test("refreshes only when the registry version is newer", () => {
+    expect(Npm.shouldRefreshInstall("1.0.0", "2.0.0")).toBe(true)
+    expect(Npm.shouldRefreshInstall("2.0.0", "2.0.0")).toBe(false)
+    expect(Npm.shouldRefreshInstall("2.0.0", "1.0.0")).toBe(false)
+    expect(Npm.shouldRefreshInstall("1.0.0-rc.1", "1.0.0")).toBe(true)
+  })
+
+  test("returns false when either version is missing", () => {
+    expect(Npm.shouldRefreshInstall(undefined, "2.0.0")).toBe(false)
+    expect(Npm.shouldRefreshInstall("1.0.0", undefined)).toBe(false)
+    expect(Npm.shouldRefreshInstall(undefined, undefined)).toBe(false)
+  })
+
+  test("falls back to string inequality for non-semver values", () => {
+    expect(Npm.shouldRefreshInstall("beta", "beta")).toBe(false)
+    expect(Npm.shouldRefreshInstall("beta", "rc")).toBe(true)
+  })
+})
+
 describe("Npm.add", () => {
+  const add = (spec: string, cache: string, options?: Npm.AddOptions) =>
+    Effect.gen(function* () {
+      const npm = yield* Npm.Service
+      return yield* npm.add(spec, options)
+    }).pipe(Effect.scoped, Effect.provide(npmLayer(cache)), Effect.runPromise)
+
   test("reifies when package cache directory exists without the package installed", async () => {
     await using tmp = await tmpdir()
     await fs.mkdir(path.join(tmp.path, "fixture-provider"))
@@ -48,12 +92,101 @@ describe("Npm.add", () => {
     const spec = `fixture-provider@file:${path.join(tmp.path, "fixture-provider")}`
     await fs.mkdir(path.join(tmp.path, "cache", "packages", Npm.sanitize(spec)), { recursive: true })
 
-    const entry = await Effect.gen(function* () {
-      const npm = yield* Npm.Service
-      return yield* npm.add(spec)
-    }).pipe(Effect.scoped, Effect.provide(npmLayer(path.join(tmp.path, "cache"))), Effect.runPromise)
+    const entry = await add(spec, path.join(tmp.path, "cache"))
+
+    expect(entry.entrypoint).toBeDefined()
+  })
+
+  test("keeps the cache for pinned file specs without touching the lockfile", async () => {
+    await using tmp = await tmpdir()
+    const cache = path.join(tmp.path, "cache")
+    const source = path.join(tmp.path, "fixture-provider")
+    const spec = `fixture-provider@file:${source}`
+    const dir = path.join(cache, "packages", Npm.sanitize(spec))
+    const cached = path.join(dir, "node_modules", "fixture-provider")
+    const lockfile = path.join(dir, "package-lock.json")
+
+    await fs.mkdir(source, { recursive: true })
+    await writePackage(source, { name: "fixture-provider", version: "1.0.0", main: "index.js" })
+    await Bun.write(path.join(source, "index.js"), "export const fixture = true\n")
+    await fs.mkdir(cached, { recursive: true })
+    await writePackage(cached, { name: "fixture-provider", version: "1.0.0", main: "index.js" })
+    await Bun.write(path.join(cached, "index.js"), "export const cached = true\n")
+    await Bun.write(lockfile, "{}")
+
+    const entry = await add(spec, cache)
+    expect(entry.entrypoint).toBeDefined()
+    expect(JSON.parse(await Bun.file(path.join(cached, "package.json")).text()).version).toBe("1.0.0")
+    expect(await Bun.file(lockfile).exists()).toBe(true)
+  })
+
+  test("falls back to the cached install when a @latest reify fails", async () => {
+    await using tmp = await tmpdir()
+    const cache = path.join(tmp.path, "cache")
+    // A name guaranteed not to resolve on the public registry so arborist reify
+    // fails at the packument fetch (before mutating node_modules), leaving the
+    // cached install intact for the fallback.
+    const spec = "opencode-test-nonexistent-9f3k2@latest"
+    const name = "opencode-test-nonexistent-9f3k2"
+    const dir = path.join(cache, "packages", Npm.sanitize(spec))
+    const cached = path.join(dir, "node_modules", name)
+    const lockfile = path.join(dir, "package-lock.json")
+
+    await fs.mkdir(cached, { recursive: true })
+    await writePackage(cached, { name, version: "1.0.0", main: "index.js" })
+    await Bun.write(path.join(cached, "index.js"), "export const cached = true\n")
+    await writePackage(dir, { dependencies: { [name]: "1.0.0" } })
+    await Bun.write(lockfile, "{}")
+
+    // `globalThis.fetch` is the only seam: `fetchRegistryVersion` calls `fetch`
+    // directly (not injectable), and arborist uses `make-fetch-happen` which is
+    // not interceptable here. The mock makes the dist-tags check report a
+    // newer version so a refresh is attempted; arborist then hits the real
+    // registry, 404s on the nonexistent name, and the cached 1.0.0 is returned
+    // instead of throwing. Works offline too (connection error also fails reify).
+    const originalFetch = globalThis.fetch
+    globalThis.fetch = (async (input: string | Request | URL) => {
+      const url = typeof input === "string" ? input : input instanceof URL ? input.href : input.url
+      if (url.endsWith(`/${name}`)) {
+        return new Response(JSON.stringify({ "dist-tags": { latest: "2.0.0" } }), { status: 200 })
+      }
+      return originalFetch(input)
+    }) as unknown as typeof fetch
+    try {
+      const entry = await add(spec, cache)
+      expect(entry.entrypoint).toBeDefined()
+      expect(await Bun.file(lockfile).exists()).toBe(false)
+      expect(JSON.parse(await Bun.file(path.join(cached, "package.json")).text()).version).toBe("1.0.0")
+    } finally {
+      globalThis.fetch = originalFetch
+    }
+  })
+
+  test("forces a cache refresh when the refresh option is set", async () => {
+    await using tmp = await tmpdir()
+    const cache = path.join(tmp.path, "cache")
+    const source = path.join(tmp.path, "fixture-provider")
+    const spec = `fixture-provider@file:${source}`
+    const dir = path.join(cache, "packages", Npm.sanitize(spec))
+    const cached = path.join(dir, "node_modules", "fixture-provider")
+
+    // source has 2.0.0
+    await fs.mkdir(source, { recursive: true })
+    await writePackage(source, { name: "fixture-provider", version: "2.0.0", main: "index.js" })
+    await Bun.write(path.join(source, "index.js"), "export const v = 2\n")
+    // cache holds a stale 1.0.0
+    await fs.mkdir(cached, { recursive: true })
+    await writePackage(cached, { name: "fixture-provider", version: "1.0.0", main: "index.js" })
+    await Bun.write(path.join(cached, "index.js"), "export const cached = true\n")
+
+    // file specs are immutable, so without `refresh` the cached 1.0.0 stays
+    await add(spec, cache)
+    expect(JSON.parse(await Bun.file(path.join(cached, "package.json")).text()).version).toBe("1.0.0")
 
+    // `refresh` forces `reify` against the current source, updating to 2.0.0
+    const entry = await add(spec, cache, { refresh: true })
     expect(entry.entrypoint).toBeDefined()
+    expect(JSON.parse(await Bun.file(path.join(cached, "package.json")).text()).version).toBe("2.0.0")
   })
 })