Merge pull request #3713 from deopa0402/fix/stale-plugin-specifier-cache

fix(auto-update): clean stale OMO plugin specifier cache roots

src/hooks/auto-update-checker/cache.ts

@@ -1,6 +1,6 @@
 import * as fs from "node:fs"
 import * as path from "node:path"
-import { CACHE_DIR, PACKAGE_NAME, getUserConfigDir } from "./constants"
+import { ACCEPTED_PACKAGE_NAMES, CACHE_DIR, PACKAGE_NAME, getUserConfigDir } from "./constants"
 import { log } from "../../shared/logger"
 
 interface BunLockfile {
@@ -12,22 +12,36 @@ interface BunLockfile {
   packages?: Record<string, unknown>
 }
 
+interface InvalidatePackageOptions {
+  acceptedPackageNames?: readonly string[]
+  cacheDir?: string
+  defaultPackageName?: string
+  userConfigDir?: string
+}
+
 function stripTrailingCommas(json: string): string {
   return json.replace(/,(\s*[}\]])/g, "$1")
 }
 
-function removeFromTextBunLock(lockPath: string, packageName: string): boolean {
+function removeFromTextBunLock(lockPath: string, packageNames: readonly string[]): boolean {
   try {
     const content = fs.readFileSync(lockPath, "utf-8")
     const lock = JSON.parse(stripTrailingCommas(content)) as BunLockfile
+    let removed = false
 
-    if (lock.packages?.[packageName]) {
-      delete lock.packages[packageName]
-      fs.writeFileSync(lockPath, JSON.stringify(lock, null, 2))
-      log(`[auto-update-checker] Removed from bun.lock: ${packageName}`)
-      return true
+    for (const packageName of packageNames) {
+      if (lock.packages?.[packageName]) {
+        delete lock.packages[packageName]
+        log(`[auto-update-checker] Removed from bun.lock: ${packageName}`)
+        removed = true
+      }
     }
-    return false
+
+    if (removed) {
+      fs.writeFileSync(lockPath, JSON.stringify(lock, null, 2))
+    }
+
+    return removed
   } catch {
     return false
   }
@@ -43,12 +57,12 @@ function deleteBinaryBunLock(lockPath: string): boolean {
   }
 }
 
-function removeFromBunLock(packageName: string): boolean {
-  const textLockPath = path.join(CACHE_DIR, "bun.lock")
-  const binaryLockPath = path.join(CACHE_DIR, "bun.lockb")
+function removeFromBunLock(cacheDir: string, packageNames: readonly string[]): boolean {
+  const textLockPath = path.join(cacheDir, "bun.lock")
+  const binaryLockPath = path.join(cacheDir, "bun.lockb")
 
   if (fs.existsSync(textLockPath)) {
-    return removeFromTextBunLock(textLockPath, packageName)
+    return removeFromTextBunLock(textLockPath, packageNames)
   }
 
   // Binary lockfiles cannot be parsed; deletion forces bun to re-resolve
@@ -59,16 +73,61 @@ function removeFromBunLock(packageName: string): boolean {
   return false
 }
 
-export function invalidatePackage(packageName: string = PACKAGE_NAME): boolean {
+function getInvalidationPackageNames(
+  packageName: string,
+  defaultPackageName: string,
+  acceptedPackageNames: readonly string[]
+): readonly string[] {
+  if (packageName === defaultPackageName) {
+    return acceptedPackageNames
+  }
+
+  return [packageName]
+}
+
+function removeSpecifierRootDirs(cacheDir: string, packageNames: readonly string[]): boolean {
+  const parentDirs = [cacheDir, path.join(cacheDir, "packages")]
+  const prefixes = packageNames.map(packageName => `${packageName}@`)
+  let removed = false
+
+  for (const parentDir of parentDirs) {
+    if (!fs.existsSync(parentDir)) {
+      continue
+    }
+
+    for (const entry of fs.readdirSync(parentDir, { withFileTypes: true })) {
+      if (!entry.isDirectory() || !prefixes.some(prefix => entry.name.startsWith(prefix))) {
+        continue
+      }
+
+      const specifierDir = path.join(parentDir, entry.name)
+      fs.rmSync(specifierDir, { recursive: true, force: true })
+      log(`[auto-update-checker] Specifier cache removed: ${specifierDir}`)
+      removed = true
+    }
+  }
+
+  return removed
+}
+
+export function invalidatePackage(
+  packageName: string = PACKAGE_NAME,
+  options: InvalidatePackageOptions = {}
+): boolean {
   try {
-    const userConfigDir = getUserConfigDir()
-    const pkgDirs = [
-      path.join(userConfigDir, "node_modules", packageName),
-      path.join(CACHE_DIR, "node_modules", packageName),
-    ]
+    const acceptedPackageNames = options.acceptedPackageNames ?? ACCEPTED_PACKAGE_NAMES
+    const cacheDir = options.cacheDir ?? CACHE_DIR
+    const defaultPackageName = options.defaultPackageName ?? PACKAGE_NAME
+    const userConfigDir = options.userConfigDir ?? getUserConfigDir()
+    const packageNames = getInvalidationPackageNames(packageName, defaultPackageName, acceptedPackageNames)
+    const pkgDirs = packageNames.flatMap(name => [
+      path.join(userConfigDir, "node_modules", name),
+      path.join(cacheDir, "node_modules", name),
+    ])
 
     let packageRemoved = false
     let lockRemoved = false
+    let specifierRemoved = false
 
     for (const pkgDir of pkgDirs) {
       if (fs.existsSync(pkgDir)) {
@@ -78,9 +137,10 @@ export function invalidatePackage(packageName: string = PACKAGE_NAME): boolean {
       }
     }
 
-    lockRemoved = removeFromBunLock(packageName)
+    specifierRemoved = removeSpecifierRootDirs(cacheDir, packageNames)
+    lockRemoved = removeFromBunLock(cacheDir, packageNames)
 
-    if (!packageRemoved && !lockRemoved) {
+    if (!packageRemoved && !specifierRemoved && !lockRemoved) {
       log(`[auto-update-checker] Package not found, nothing to invalidate: ${packageName}`)
       return false
     }

src/hooks/auto-update-checker/checker/cached-version.test.ts

@@ -1,7 +1,8 @@
-import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test"
+import { afterEach, beforeEach, describe, expect, it } from "bun:test"
 import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"
 import { tmpdir } from "node:os"
 import { join } from "node:path"
+import { getCachedVersion } from "./cached-version"
 
 // Hold mutable mock state so beforeEach can swap the cache root for each test.
 const mockState: { candidates: string[]; walkUpResult: string | null } = {
@@ -9,27 +10,14 @@ const mockState: { candidates: string[]; walkUpResult: string | null } = {
   walkUpResult: null,
 }
 
-mock.module("../constants", () => ({
-  INSTALLED_PACKAGE_JSON_CANDIDATES: new Proxy([], {
-    get(_, prop) {
-      const current = mockState.candidates
-      // Forward array methods/properties to the mutable candidates list
-      // so getCachedVersion's `for (... of ...)` sees fresh data per test.
-      const value = (unsafeTestValue<Record<PropertyKey, unknown>>(current))[prop]
-      if (typeof value === "function") {
-        return (value as (...args: unknown[]) => unknown).bind(current)
-      }
-      return value
-    },
-  }),
-}))
-
-mock.module("./package-json-locator", () => ({
-  findPackageJsonUp: () => mockState.walkUpResult,
-}))
-
-import { getCachedVersion } from "./cached-version"
-import { unsafeTestValue } from "../../../../test-support/unsafe-test-value"
+function getIsolatedCachedVersion(): string | null {
+  return getCachedVersion({
+    packageJsonCandidates: mockState.candidates,
+    findPackageJson: () => null,
+    currentDir: null,
+    execDir: null,
+  })
+}
 
 describe("getCachedVersion (GH-3257)", () => {
   let cacheRoot: string
@@ -54,7 +42,7 @@ describe("getCachedVersion (GH-3257)", () => {
     mkdirSync(pkgDir, { recursive: true })
     writeFileSync(join(pkgDir, "package.json"), JSON.stringify({ name: "oh-my-opencode", version: "3.16.0" }))
 
-    expect(getCachedVersion()).toBe("3.16.0")
+    expect(getIsolatedCachedVersion()).toBe("3.16.0")
   })
 
   it("returns the version when the package is installed under oh-my-openagent", () => {
@@ -65,7 +53,7 @@ describe("getCachedVersion (GH-3257)", () => {
     mkdirSync(pkgDir, { recursive: true })
     writeFileSync(join(pkgDir, "package.json"), JSON.stringify({ name: "oh-my-openagent", version: "3.16.0" }))
 
-    expect(getCachedVersion()).toBe("3.16.0")
+    expect(getIsolatedCachedVersion()).toBe("3.16.0")
   })
 
   it("prefers oh-my-opencode when both are installed", () => {
@@ -77,11 +65,11 @@ describe("getCachedVersion (GH-3257)", () => {
     mkdirSync(aliasDir, { recursive: true })
     writeFileSync(join(aliasDir, "package.json"), JSON.stringify({ name: "oh-my-openagent", version: "3.15.0" }))
 
-    expect(getCachedVersion()).toBe("3.16.0")
+    expect(getIsolatedCachedVersion()).toBe("3.16.0")
   })
 
   it("returns null when neither candidate exists and fallbacks find nothing", () => {
-    expect(getCachedVersion()).toBeNull()
+    expect(getIsolatedCachedVersion()).toBeNull()
   })
 
   it("prefers the loaded module's package.json over flat-install candidates", () => {
@@ -100,6 +88,13 @@ describe("getCachedVersion (GH-3257)", () => {
     mkdirSync(flatDir, { recursive: true })
     writeFileSync(join(flatDir, "package.json"), JSON.stringify({ name: "oh-my-opencode", version: "3.17.6" }))
 
-    expect(getCachedVersion()).toBe("3.17.5")
+    expect(
+      getCachedVersion({
+        packageJsonCandidates: mockState.candidates,
+        findPackageJson: () => mockState.walkUpResult,
+        currentDir: sandboxDir,
+        execDir: null,
+      })
+    ).toBe("3.17.5")
   })
 })

src/hooks/auto-update-checker/checker/cached-version.ts

@@ -6,13 +6,23 @@ import type { PackageJson } from "../types"
 import { INSTALLED_PACKAGE_JSON_CANDIDATES } from "../constants"
 import { findPackageJsonUp } from "./package-json-locator"
 
+interface CachedVersionOptions {
+  packageJsonCandidates?: readonly string[]
+  findPackageJson?: (startPath: string) => string | null
+  currentDir?: string | null
+  execDir?: string | null
+}
+
 function readPackageVersion(packageJsonPath: string): string | null {
   const content = fs.readFileSync(packageJsonPath, "utf-8")
   const pkg = JSON.parse(content) as PackageJson
   return pkg.version ?? null
 }
 
-export function getCachedVersion(): string | null {
+export function getCachedVersion(options: CachedVersionOptions = {}): string | null {
+  const packageJsonCandidates = options.packageJsonCandidates ?? INSTALLED_PACKAGE_JSON_CANDIDATES
+  const findPackageJson = options.findPackageJson ?? findPackageJsonUp
+
   // Walk up from the loaded module first. OpenCode loads plugins from a
   // per-plugin sandbox at <CACHE_DIR>/<plugin-entry>/node_modules/<pkg>/, while
   // a parallel flat install at <CACHE_DIR>/node_modules/<pkg>/ can drift
@@ -20,16 +30,18 @@ export function getCachedVersion(): string | null {
   // first means the toast can announce a version the runtime isn't running.
   // The module-relative walk-up always reflects what is actually loaded.
   try {
-    const currentDir = path.dirname(fileURLToPath(import.meta.url))
-    const pkgPath = findPackageJsonUp(currentDir)
-    if (pkgPath) {
-      return readPackageVersion(pkgPath)
+    const currentDir = options.currentDir === undefined ? path.dirname(fileURLToPath(import.meta.url)) : options.currentDir
+    if (currentDir) {
+      const pkgPath = findPackageJson(currentDir)
+      if (pkgPath) {
+        return readPackageVersion(pkgPath)
+      }
     }
   } catch (err) {
     log("[auto-update-checker] Failed to resolve version from current directory:", err)
   }
 
-  for (const candidate of INSTALLED_PACKAGE_JSON_CANDIDATES) {
+  for (const candidate of packageJsonCandidates) {
     try {
       if (fs.existsSync(candidate)) {
         return readPackageVersion(candidate)
@@ -40,10 +52,12 @@ export function getCachedVersion(): string | null {
   }
 
   try {
-    const execDir = path.dirname(fs.realpathSync(process.execPath))
-    const pkgPath = findPackageJsonUp(execDir)
-    if (pkgPath) {
-      return readPackageVersion(pkgPath)
+    const execDir = options.execDir === undefined ? path.dirname(fs.realpathSync(process.execPath)) : options.execDir
+    if (execDir) {
+      const pkgPath = findPackageJson(execDir)
+      if (pkgPath) {
+        return readPackageVersion(pkgPath)
+      }
     }
   } catch (err) {
     log("[auto-update-checker] Failed to resolve version from execPath:", err)

src/hooks/zauc-mocks-cache/cache.test.ts

@@ -1,38 +1,19 @@
-import { afterAll, afterEach, beforeEach, describe, expect, it, mock } from "bun:test"
+import { afterEach, beforeEach, describe, expect, it } from "bun:test"
 import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"
 import { join } from "node:path"
+import { invalidatePackage } from "../auto-update-checker/cache"
 
 const TEST_CACHE_DIR = join(import.meta.dir, "__test-cache__")
 const TEST_OPENCODE_CACHE_DIR = join(TEST_CACHE_DIR, "opencode")
 const TEST_USER_CONFIG_DIR = "/tmp/opencode-config"
 
-let importCounter = 0
-
-// Capture real modules BEFORE mocking
-const _realConstants = require("../auto-update-checker/constants")
-const _realLogger = require("../../shared/logger")
-
-async function importFreshCacheModule(): Promise<typeof import("../auto-update-checker/cache")> {
-  mock.module("../auto-update-checker/constants", () => ({
-    CACHE_DIR: TEST_OPENCODE_CACHE_DIR,
-    PACKAGE_NAME: "oh-my-opencode",
-    NPM_REGISTRY_URL: "https://registry.npmjs.org/-/package/oh-my-opencode/dist-tags",
-    NPM_FETCH_TIMEOUT: 5000,
-    VERSION_FILE: join(TEST_OPENCODE_CACHE_DIR, "version"),
-    INSTALLED_PACKAGE_JSON: join(TEST_OPENCODE_CACHE_DIR, "node_modules", "oh-my-opencode", "package.json"),
-    getUserConfigDir: () => TEST_USER_CONFIG_DIR,
-    getUserOpencodeConfig: () => join(TEST_USER_CONFIG_DIR, "opencode.json"),
-    getUserOpencodeConfigJsonc: () => join(TEST_USER_CONFIG_DIR, "opencode.jsonc"),
-    getWindowsAppdataDir: () => null,
-  }))
-
-  mock.module("../../shared/logger", () => ({
-    log: () => {},
-  }))
-
-  const cacheModule = await import(`../auto-update-checker/cache?test=${importCounter++}`)
-  mock.restore()
-  return cacheModule
+function testInvalidatePackage(packageName?: string): boolean {
+  return invalidatePackage(packageName, {
+    acceptedPackageNames: ["oh-my-opencode", "oh-my-openagent"],
+    cacheDir: TEST_OPENCODE_CACHE_DIR,
+    defaultPackageName: "oh-my-opencode",
+    userConfigDir: TEST_USER_CONFIG_DIR,
+  })
 }
 
 function resetTestCache(): void {
@@ -56,6 +37,8 @@ function resetTestCache(): void {
         },
         packages: {
           "oh-my-opencode": {},
+          "oh-my-openagent": {},
+          "some-other-package": {},
           other: {},
         },
       },
@@ -81,12 +64,28 @@ describe("invalidatePackage", () => {
   })
 
   it("invalidates the installed package from the OpenCode cache directory", async () => {
-    const { invalidatePackage } = await importFreshCacheModule()
+    const rootSpecifierDir = join(TEST_OPENCODE_CACHE_DIR, "oh-my-opencode@latest")
+    const rootAcceptedSpecifierDir = join(TEST_OPENCODE_CACHE_DIR, "oh-my-openagent@latest")
+    const packagesSpecifierDir = join(TEST_OPENCODE_CACHE_DIR, "packages", "oh-my-opencode@latest")
+    const packagesAcceptedSpecifierDir = join(TEST_OPENCODE_CACHE_DIR, "packages", "oh-my-openagent@latest")
+    const otherSpecifierDir = join(TEST_OPENCODE_CACHE_DIR, "packages", "other@latest")
+    mkdirSync(join(TEST_OPENCODE_CACHE_DIR, "node_modules", "oh-my-openagent"), { recursive: true })
+    mkdirSync(join(rootSpecifierDir, "node_modules", "oh-my-opencode"), { recursive: true })
+    mkdirSync(join(rootAcceptedSpecifierDir, "node_modules", "oh-my-openagent"), { recursive: true })
+    mkdirSync(join(packagesSpecifierDir, "node_modules", "oh-my-opencode"), { recursive: true })
+    mkdirSync(join(packagesAcceptedSpecifierDir, "node_modules", "oh-my-openagent"), { recursive: true })
+    mkdirSync(otherSpecifierDir, { recursive: true })
 
-    const result = invalidatePackage()
+    const result = testInvalidatePackage()
 
     expect(result).toBe(true)
+    expect(existsSync(rootSpecifierDir)).toBe(false)
+    expect(existsSync(rootAcceptedSpecifierDir)).toBe(false)
+    expect(existsSync(packagesSpecifierDir)).toBe(false)
+    expect(existsSync(packagesAcceptedSpecifierDir)).toBe(false)
+    expect(existsSync(otherSpecifierDir)).toBe(true)
     expect(existsSync(join(TEST_OPENCODE_CACHE_DIR, "node_modules", "oh-my-opencode"))).toBe(false)
+    expect(existsSync(join(TEST_OPENCODE_CACHE_DIR, "node_modules", "oh-my-openagent"))).toBe(false)
 
     const packageJson = JSON.parse(readFileSync(join(TEST_OPENCODE_CACHE_DIR, "package.json"), "utf-8")) as {
       dependencies?: Record<string, string>
@@ -101,12 +100,25 @@ describe("invalidatePackage", () => {
     expect(bunLock.workspaces?.[""]?.dependencies?.["oh-my-opencode"]).toBe("latest")
     expect(bunLock.workspaces?.[""]?.dependencies?.other).toBe("1.0.0")
     expect(bunLock.packages?.["oh-my-opencode"]).toBeUndefined()
+    expect(bunLock.packages?.["oh-my-openagent"]).toBeUndefined()
+    expect(bunLock.packages?.["some-other-package"]).toEqual({})
     expect(bunLock.packages?.other).toEqual({})
+
+    const explicitSpecifierDir = join(TEST_OPENCODE_CACHE_DIR, "some-other-package@latest")
+    const acceptedSpecifierDir = join(TEST_OPENCODE_CACHE_DIR, "oh-my-openagent@beta")
+    mkdirSync(explicitSpecifierDir, { recursive: true })
+    mkdirSync(acceptedSpecifierDir, { recursive: true })
+
+    const explicitResult = testInvalidatePackage("some-other-package")
+
+    expect(explicitResult).toBe(true)
+    expect(existsSync(explicitSpecifierDir)).toBe(false)
+    expect(existsSync(acceptedSpecifierDir)).toBe(true)
+
+    const explicitBunLock = JSON.parse(readFileSync(join(TEST_OPENCODE_CACHE_DIR, "bun.lock"), "utf-8")) as {
+      packages?: Record<string, unknown>
+    }
+    expect(explicitBunLock.packages?.["some-other-package"]).toBeUndefined()
+    expect(explicitBunLock.packages?.other).toEqual({})
   })
 })
-
-afterAll(() => {
-  mock.module("../auto-update-checker/constants", () => _realConstants)
-  mock.module("../../shared/logger", () => _realLogger)
-  mock.restore()
-})