From 4d108c195119cccd357f3d26e6b4a7b772cbb4ba Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 29 May 2026 06:34:10 +0000 Subject: [PATCH] fix(plugins): enumerate linked plugins for sub-discovery PluginManager.link symlinks the package into /node_modules and records it in omp-plugins.lock.json, but never writes to /package.json#dependencies. getEnabledPlugins iterated only the dependency map, so the documented `omp install ./local-extension` workflow (delegated to plugin link) succeeded but its sibling skills/, hooks/, tools/, etc. stayed invisible after install. Iterate the union of package.json#dependencies and omp-plugins.lock.json#plugins so symlinked-only packages surface alongside npm/marketplace installs. Lockfile entries whose node_modules tree has since been deleted (stale link) are skipped silently. Linked-only setups with no /package.json at all now work too. Per-PR review feedback: https://github.com/can1357/oh-my-pi/pull/1498 --- .../src/extensibility/plugins/loader.ts | 47 ++++++++++++------- .../test/discovery/omp-plugins.test.ts | 28 +++++++++++ 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/packages/coding-agent/src/extensibility/plugins/loader.ts b/packages/coding-agent/src/extensibility/plugins/loader.ts index af85e12ef..fac12a8c9 100644 --- a/packages/coding-agent/src/extensibility/plugins/loader.ts +++ b/packages/coding-agent/src/extensibility/plugins/loader.ts @@ -52,48 +52,63 @@ async function loadProjectOverrides(cwd: string): Promise/package.json#dependencies` (`bun install`-installed + * packages) and `/omp-plugins.lock.json#plugins` (so locally + * `plugin link`-symlinked extensions, which never get a dependency entry, + * are still discovered). The optional `home` parameter pins the plugins + * root for callers that need to enumerate plugins relative to a non-default + * home (tests with a tempdir, discovery loaders threaded with + * `LoadContext.home`). */ export async function getEnabledPlugins(cwd: string, opts: { home?: string } = {}): Promise { const { home } = opts; - const pkgJsonPath = getPluginsPackageJson(home); - let pkg: { dependencies?: Record }; - try { - pkg = await Bun.file(pkgJsonPath).json(); - } catch (err) { - if (isEnoent(err)) return []; - throw err; - } const nodeModulesPath = getPluginsNodeModules(home); if (!fs.existsSync(nodeModulesPath)) { return []; } - const deps = pkg.dependencies || {}; + let depsKeys: string[] = []; + const pkgJsonPath = getPluginsPackageJson(home); + try { + const pkg: { dependencies?: Record } = await Bun.file(pkgJsonPath).json(); + depsKeys = Object.keys(pkg.dependencies ?? {}); + } catch (err) { + // Linked-only setups may have no `/package.json` yet — that's + // fine, the lockfile still records the link. + if (!isEnoent(err)) throw err; + } + const runtimeConfig = await loadRuntimeConfig(home); const projectOverrides = await loadProjectOverrides(cwd); + + // Union: dependencies (npm/marketplace installs) ∪ runtime-config plugins + // (links + already-recorded installs). Set preserves first-seen order, + // putting deps before link-only entries for deterministic output. + const names = new Set(depsKeys); + for (const name of Object.keys(runtimeConfig.plugins ?? {})) { + names.add(name); + } + const plugins: InstalledPlugin[] = []; - for (const [name] of Object.entries(deps)) { + for (const name of names) { const pluginPkgPath = path.join(nodeModulesPath, name, "package.json"); let pluginPkg: { version: string; omp?: PluginManifest; pi?: PluginManifest }; try { pluginPkg = await Bun.file(pluginPkgPath).json(); } catch (err) { + // Lockfile entry without a corresponding node_modules tree means the + // link was deleted out from under us; skip silently. if (isEnoent(err)) continue; throw err; } const manifest: PluginManifest | undefined = pluginPkg.omp || pluginPkg.pi; - if (!manifest) { // Not an omp plugin, skip continue; } - manifest.version = pluginPkg.version; const runtimeState = runtimeConfig.plugins[name]; diff --git a/packages/coding-agent/test/discovery/omp-plugins.test.ts b/packages/coding-agent/test/discovery/omp-plugins.test.ts index 0c84de4d6..f091cd50a 100644 --- a/packages/coding-agent/test/discovery/omp-plugins.test.ts +++ b/packages/coding-agent/test/discovery/omp-plugins.test.ts @@ -215,3 +215,31 @@ test("disabled installed plugins do not contribute sub-discovery", async () => { const skills = await loadFromPlugin<{ name: string; path: string }>(skillCapability.id, ctx()); expect(skills.find(s => s.path.includes("my-disabled-ext"))).toBeUndefined(); }); + +test("linked plugins (only in lockfile, not in package.json#dependencies) are surfaced", async () => { + // `omp plugin link ./local-ext` creates a symlink under + // `/node_modules/` plus a lockfile entry, but it never + // touches `/package.json#dependencies`. The discovery path must + // still find the package — otherwise the documented `omp install + // ./local-extension` workflow leaves the sibling skills/hooks/tools + // invisible (see PR #1498 review). + const pluginsDir = path.join(home, ".omp", "plugins"); + const nodeModules = path.join(pluginsDir, "node_modules"); + fs.mkdirSync(nodeModules, { recursive: true }); + const linkTarget = path.join(nodeModules, "my-linked-ext"); + fs.symlinkSync(ext, linkTarget); + // Intentionally NO `/package.json` — matches a fresh `plugin link` + // against a setup that has never run `plugin install`. + writeFile( + path.join(pluginsDir, "omp-plugins.lock.json"), + JSON.stringify({ + plugins: { "my-linked-ext": { version: "1.0.0", enabled: true, enabledFeatures: null } }, + settings: {}, + }), + ); + + const skills = await loadFromPlugin<{ name: string; path: string }>(skillCapability.id, ctx()); + const tools = await loadFromPlugin<{ name: string; path: string }>(toolCapability.id, ctx()); + expect(skills.find(s => s.name === "my-skill" && s.path.includes("my-linked-ext"))).toBeDefined(); + expect(tools.find(t => t.name === "wcount" && t.path.includes("my-linked-ext"))).toBeDefined(); +});