fix(coding-agent): cached plugin extension resolution
- Added shared plugin cache invalidators wired through clearClaudePluginRootsCache.\n- Memoized enabled plugin discovery and legacy bare dependency fallback resolution.\n- Added regression coverage for cache reuse and invalidation.\n\nFixes #4197
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -830,6 +830,13 @@ export async function resolveOrDefaultProjectRegistryPath(cwd: string): Promise<
|
||||
|
||||
const pluginRootsCache = new Map<string, { roots: ClaudePluginRoot[]; warnings: string[] }>();
|
||||
|
||||
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) {
|
||||
|
||||
@@ -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<string, string | null>();
|
||||
const packageImportsCache = new Map<string, Record<string, unknown> | null>();
|
||||
const nodePackageRootCache = new Map<string, Promise<string | null>>();
|
||||
const packageManifestCache = new Map<string, Promise<Record<string, unknown> | null>>();
|
||||
const bareDependencyResolutionCache = new Map<string, Promise<string | null>>();
|
||||
const realpathCache = new Map<string, Promise<string>>();
|
||||
|
||||
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<string | null> {
|
||||
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<string | null> {
|
||||
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<Record<string, unknown> | 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<Record<string, unknown> | 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<string | null> {
|
||||
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<string>();
|
||||
|
||||
/** Resolve symlinks in a path, falling back to the input if realpath fails. */
|
||||
async function realpathOrSelf(p: string): Promise<string> {
|
||||
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<string> {
|
||||
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();
|
||||
}
|
||||
|
||||
@@ -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<string, Promise<ScopedInstalledPlugin[]>>();
|
||||
|
||||
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<ScopedInstalledPlugin[]> {
|
||||
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<ScopedInstalledPlugin[]> {
|
||||
const projectOverrides = await loadProjectOverrides(cwd);
|
||||
|
||||
const userRoot = getPluginsDir(home);
|
||||
|
||||
@@ -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<void> {
|
||||
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");
|
||||
});
|
||||
Reference in New Issue
Block a user