diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 11098023c..5aec9ddfc 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed plugin and legacy extension discovery repeatedly re-reading plugin manifests and walking extension `node_modules` by caching results until plugin cache invalidation. ([#4197](https://github.com/can1357/oh-my-pi/issues/4197)) + ## [16.3.0] - 2026-07-02 ### Added diff --git a/packages/coding-agent/src/discovery/helpers.ts b/packages/coding-agent/src/discovery/helpers.ts index 95138d3ff..d1e6e7c91 100644 --- a/packages/coding-agent/src/discovery/helpers.ts +++ b/packages/coding-agent/src/discovery/helpers.ts @@ -830,6 +830,13 @@ export async function resolveOrDefaultProjectRegistryPath(cwd: string): Promise< const pluginRootsCache = new Map(); +const pluginCacheInvalidators = new Set<() => void>(); + +/** Register a process-global plugin cache invalidator called whenever plugin roots are cleared. */ +export function registerPluginCacheInvalidator(invalidator: () => void): void { + pluginCacheInvalidators.add(invalidator); +} + /** * List all installed Claude Code plugin roots from the plugin cache. * Reads ~/.claude/plugins/installed_plugins.json and ~/.omp/plugins/installed_plugins.json, @@ -1009,6 +1016,7 @@ export async function listClaudePluginRoots( */ export function clearClaudePluginRootsCache(): void { pluginRootsCache.clear(); + for (const invalidate of pluginCacheInvalidators) invalidate(); preloadedPluginRoots = [...injectedPluginDirRoots]; // Re-warm preloaded roots asynchronously so sync LSP config reads stay valid if (lastPreloadHome) { diff --git a/packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts b/packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts index 4335665ec..005ade523 100644 --- a/packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts +++ b/packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts @@ -3,6 +3,7 @@ import { isBuiltin } from "node:module"; import * as path from "node:path"; import * as url from "node:url"; import { isCompiledBinary, stripWindowsExtendedLengthPathPrefix } from "@oh-my-pi/pi-utils"; +import { registerPluginCacheInvalidator } from "../../discovery/helpers"; import { BUNDLED_PI_REGISTRY_KEYS } from "./legacy-pi-bundled-keys"; const IS_COMPILED_BINARY = isCompiledBinary(); @@ -202,6 +203,22 @@ const SOURCE_MODULE_EXTENSIONS = [".ts", ".tsx", ".mts", ".cts", ".js", ".jsx", const SUPPORTED_PACKAGE_IMPORT_CONDITIONS = new Set(["bun", "node", "import", "default"]); const packageRootCache = new Map(); const packageImportsCache = new Map | null>(); +const nodePackageRootCache = new Map>(); +const packageManifestCache = new Map | null>>(); +const bareDependencyResolutionCache = new Map>(); +const realpathCache = new Map>(); + +function clearLegacyPiResolutionCaches(): void { + resolvedSpecifierFallbacks.clear(); + packageRootCache.clear(); + packageImportsCache.clear(); + nodePackageRootCache.clear(); + packageManifestCache.clear(); + bareDependencyResolutionCache.clear(); + realpathCache.clear(); +} + +registerPluginCacheInvalidator(clearLegacyPiResolutionCaches); const PACKAGE_IMPORT_EXCLUDED = Symbol("packageImportExcluded"); // Extensions that imported TypeBox directly used to resolve against a real @@ -730,6 +747,16 @@ function splitBarePackageSpecifier(specifier: string): BarePackageSpecifier | nu } async function findNodePackageRoot(packageName: string, importerPath: string): Promise { + const cacheKey = `${packageName}\0${path.resolve(path.dirname(importerPath))}`; + const cached = nodePackageRootCache.get(cacheKey); + if (cached) return cached; + + const promise = findNodePackageRootUncached(packageName, importerPath); + nodePackageRootCache.set(cacheKey, promise); + return promise; +} + +async function findNodePackageRootUncached(packageName: string, importerPath: string): Promise { let dir = path.dirname(importerPath); while (true) { const candidate = path.join(dir, "node_modules", packageName); @@ -745,6 +772,15 @@ async function findNodePackageRoot(packageName: string, importerPath: string): P } async function readPackageManifest(packageRoot: string): Promise | null> { + const cached = packageManifestCache.get(packageRoot); + if (cached) return cached; + + const promise = readPackageManifestUncached(packageRoot); + packageManifestCache.set(packageRoot, promise); + return promise; +} + +async function readPackageManifestUncached(packageRoot: string): Promise | null> { try { const manifest = await Bun.file(path.join(packageRoot, "package.json")).json(); return isRecord(manifest) ? manifest : null; @@ -841,6 +877,17 @@ async function resolveExtensionBareDependency(specifier: string, importerPath: s if (!isBareExtensionDependencySpecifier(specifier)) { return null; } + + const cacheKey = `${specifier}\0${path.resolve(path.dirname(importerPath))}`; + const cached = bareDependencyResolutionCache.get(cacheKey); + if (cached) return cached; + + const promise = resolveExtensionBareDependencyUncached(specifier, importerPath); + bareDependencyResolutionCache.set(cacheKey, promise); + return promise; +} + +async function resolveExtensionBareDependencyUncached(specifier: string, importerPath: string): Promise { try { const resolved = Bun.resolveSync(specifier, path.dirname(importerPath)); if (resolved && resolved !== specifier && !resolved.startsWith("node:") && !resolved.startsWith("bun:")) { @@ -892,6 +939,15 @@ const hookedExtensionEntries = new Set(); /** Resolve symlinks in a path, falling back to the input if realpath fails. */ async function realpathOrSelf(p: string): Promise { + const cached = realpathCache.get(p); + if (cached) return cached; + + const promise = realpathOrSelfUncached(p); + realpathCache.set(p, promise); + return promise; +} + +async function realpathOrSelfUncached(p: string): Promise { try { return await fs.promises.realpath(p); } catch { @@ -1061,5 +1117,5 @@ export function installLegacyPiSpecifierShim(): void { /** Test seam: clears the memoized canonical specifier resolutions. */ export function __resetLegacyPiResolutionCache(): void { - resolvedSpecifierFallbacks.clear(); + clearLegacyPiResolutionCaches(); } diff --git a/packages/coding-agent/src/extensibility/plugins/loader.ts b/packages/coding-agent/src/extensibility/plugins/loader.ts index 17855205c..cd1d62a90 100644 --- a/packages/coding-agent/src/extensibility/plugins/loader.ts +++ b/packages/coding-agent/src/extensibility/plugins/loader.ts @@ -8,7 +8,7 @@ import * as fs from "node:fs"; import * as path from "node:path"; import { getPluginsDir, getPluginsLockfile, isEnoent } from "@oh-my-pi/pi-utils"; import { getConfigDirPaths } from "../../config"; -import { resolveActiveProjectRegistryPath } from "../../discovery/helpers"; +import { registerPluginCacheInvalidator, resolveActiveProjectRegistryPath } from "../../discovery/helpers"; import { installLegacyPiSpecifierShim } from "./legacy-pi-compat"; import { normalizePluginRuntimeConfig } from "./runtime-config"; import type { InstalledPlugin, PluginManifest, PluginRuntimeConfig, ProjectPluginOverrides } from "./types"; @@ -20,6 +20,18 @@ export interface ScopedInstalledPlugin extends InstalledPlugin { installLegacyPiSpecifierShim(); +const enabledPluginsCache = new Map>(); + +function enabledPluginsCacheKey(cwd: string, home?: string): string { + return `${path.resolve(cwd)}\0${home === undefined ? "" : path.resolve(home)}`; +} + +function clearEnabledPluginsCache(): void { + enabledPluginsCache.clear(); +} + +registerPluginCacheInvalidator(clearEnabledPluginsCache); + // ============================================================================= // Runtime Config Loading // ============================================================================= @@ -163,6 +175,23 @@ async function collectPluginsAtRoot( */ export async function getEnabledPlugins(cwd: string, opts: { home?: string } = {}): Promise { const { home } = opts; + const cacheKey = enabledPluginsCacheKey(cwd, home); + const cached = enabledPluginsCache.get(cacheKey); + if (cached) return cached; + + const loadPromise = loadEnabledPlugins(cwd, home); + enabledPluginsCache.set(cacheKey, loadPromise); + try { + return await loadPromise; + } catch (err) { + if (enabledPluginsCache.get(cacheKey) === loadPromise) { + enabledPluginsCache.delete(cacheKey); + } + throw err; + } +} + +async function loadEnabledPlugins(cwd: string, home?: string): Promise { const projectOverrides = await loadProjectOverrides(cwd); const userRoot = getPluginsDir(home); diff --git a/packages/coding-agent/test/issue-4197-plugin-resolution-cache.test.ts b/packages/coding-agent/test/issue-4197-plugin-resolution-cache.test.ts new file mode 100644 index 000000000..bddefaec2 --- /dev/null +++ b/packages/coding-agent/test/issue-4197-plugin-resolution-cache.test.ts @@ -0,0 +1,93 @@ +import { afterEach, expect, mock, spyOn, test } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { clearClaudePluginRootsCache } from "@oh-my-pi/pi-coding-agent/discovery/helpers"; +import { + __resetLegacyPiResolutionCache, + __rewriteLegacyExtensionSourceForTests, +} from "@oh-my-pi/pi-coding-agent/extensibility/plugins/legacy-pi-compat"; +import { getEnabledPlugins } from "@oh-my-pi/pi-coding-agent/extensibility/plugins/loader"; +import { removeWithRetries } from "@oh-my-pi/pi-utils"; + +const tempRoots: string[] = []; + +afterEach(async () => { + __resetLegacyPiResolutionCache(); + clearClaudePluginRootsCache(); + mock.restore(); + for (const root of tempRoots.splice(0)) { + await removeWithRetries(root); + } +}); + +async function writeJson(filePath: string, value: unknown): Promise { + await Bun.write(filePath, `${JSON.stringify(value)}\n`); +} + +test("getEnabledPlugins caches repeated discovery for the same cwd and home until plugin caches clear", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "omp-plugin-cache-")); + tempRoots.push(root); + const home = path.join(root, "home"); + const cwd = path.join(root, "project"); + const pluginsDir = path.join(home, ".omp", "plugins"); + const pluginPackageJson = path.join(pluginsDir, "node_modules", "omp-cache-repro", "package.json"); + await fs.mkdir(path.dirname(pluginPackageJson), { recursive: true }); + await fs.mkdir(cwd, { recursive: true }); + await writeJson(path.join(pluginsDir, "package.json"), { dependencies: { "omp-cache-repro": "1.0.0" } }); + await writeJson(path.join(pluginsDir, "omp-plugins.lock.json"), { + plugins: { "omp-cache-repro": { version: "1.0.0", enabled: true, enabledFeatures: null } }, + settings: {}, + }); + await writeJson(pluginPackageJson, { + name: "omp-cache-repro", + version: "1.0.0", + omp: { tools: "tools" }, + }); + + const [firstPlugin] = await getEnabledPlugins(cwd, { home }); + await writeJson(pluginPackageJson, { + name: "omp-cache-repro", + version: "2.0.0", + omp: { tools: "tools" }, + }); + const [cachedPlugin] = await getEnabledPlugins(cwd, { home }); + + expect(firstPlugin?.version).toBe("1.0.0"); + expect(cachedPlugin?.version).toBe("1.0.0"); + + clearClaudePluginRootsCache(); + const [refreshedPlugin] = await getEnabledPlugins(cwd, { home }); + + expect(refreshedPlugin?.version).toBe("2.0.0"); +}); + +test("legacy bare dependency rewrites cache fallback package resolution until plugin caches clear", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "omp-legacy-cache-")); + tempRoots.push(root); + const importer = path.join(root, "extension", "src", "entry.ts"); + const depRoot = path.join(root, "extension", "node_modules", "left-pad"); + const manifestPath = path.join(depRoot, "package.json"); + await fs.mkdir(path.dirname(importer), { recursive: true }); + await fs.mkdir(depRoot, { recursive: true }); + await Bun.write(importer, "export {};\n"); + await writeJson(manifestPath, { name: "left-pad", version: "1.0.0", main: "index.js" }); + await Bun.write(path.join(depRoot, "index.js"), "export default function leftPad() {}\n"); + await Bun.write(path.join(depRoot, "alt.js"), "export default function altLeftPad() {}\n"); + + spyOn(Bun, "resolveSync").mockImplementation(() => { + throw new Error("compiled fallback"); + }); + + const firstRewrite = await __rewriteLegacyExtensionSourceForTests('import leftPad from "left-pad";', importer); + await writeJson(manifestPath, { name: "left-pad", version: "1.0.0", main: "alt.js" }); + const cachedRewrite = await __rewriteLegacyExtensionSourceForTests('import leftPad from "left-pad";', importer); + + expect(firstRewrite).toContain("index.js"); + expect(cachedRewrite).toBe(firstRewrite); + + clearClaudePluginRootsCache(); + const refreshedRewrite = await __rewriteLegacyExtensionSourceForTests('import leftPad from "left-pad";', importer); + + expect(refreshedRewrite).toContain("alt.js"); +});