From cc5168b5c31ff21aaf50263d90aac65ae17cacd6 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 13 Mar 2026 11:24:40 -0700 Subject: [PATCH] Fix plugin update dependency failures and dedupe warnings --- src/infra/install-package-dir.test.ts | 54 +++++++++++++++++++++++++++ src/infra/install-package-dir.ts | 4 +- src/plugins/loader.test.ts | 24 ++++++++++++ src/plugins/loader.ts | 8 ++++ 4 files changed, 89 insertions(+), 1 deletion(-) diff --git a/src/infra/install-package-dir.test.ts b/src/infra/install-package-dir.test.ts index 1386f6074fa..cacbcadf5cc 100644 --- a/src/infra/install-package-dir.test.ts +++ b/src/infra/install-package-dir.test.ts @@ -3,8 +3,17 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { runCommandWithTimeout } from "../process/exec.js"; import { installPackageDir } from "./install-package-dir.js"; +vi.mock("../process/exec.js", async () => { + const actual = await vi.importActual("../process/exec.js"); + return { + ...actual, + runCommandWithTimeout: vi.fn(actual.runCommandWithTimeout), + }; +}); + async function listMatchingDirs(root: string, prefix: string): Promise { const entries = await fs.readdir(root, { withFileTypes: true }); return entries @@ -263,4 +272,49 @@ describe("installPackageDir", () => { const backupRoot = path.join(preservedInstallRoot, ".openclaw-install-backups"); await expect(fs.readdir(backupRoot)).resolves.toHaveLength(1); }); + + it("installs peer dependencies for isolated plugin package installs", async () => { + fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-install-package-dir-")); + const sourceDir = path.join(fixtureRoot, "source"); + const targetDir = path.join(fixtureRoot, "plugins", "demo"); + await fs.mkdir(sourceDir, { recursive: true }); + await fs.writeFile( + path.join(sourceDir, "package.json"), + JSON.stringify({ + name: "demo-plugin", + version: "1.0.0", + dependencies: { + zod: "^4.0.0", + }, + }), + "utf-8", + ); + + vi.mocked(runCommandWithTimeout).mockResolvedValue({ + stdout: "", + stderr: "", + code: 0, + signal: null, + killed: false, + termination: "exit", + }); + + const result = await installPackageDir({ + sourceDir, + targetDir, + mode: "install", + timeoutMs: 1_000, + copyErrorPrefix: "failed to copy plugin", + hasDeps: true, + depsLogMessage: "Installing deps…", + }); + + expect(result).toEqual({ ok: true }); + expect(vi.mocked(runCommandWithTimeout)).toHaveBeenCalledWith( + ["npm", "install", "--omit=dev", "--silent", "--ignore-scripts"], + expect.objectContaining({ + cwd: expect.stringContaining(".openclaw-install-stage-"), + }), + ); + }); }); diff --git a/src/infra/install-package-dir.ts b/src/infra/install-package-dir.ts index 17878599160..45611b17ffe 100644 --- a/src/infra/install-package-dir.ts +++ b/src/infra/install-package-dir.ts @@ -189,7 +189,9 @@ export async function installPackageDir(params: { await sanitizeManifestForNpmInstall(stageDir); params.logger?.info?.(params.depsLogMessage); const npmRes = await runCommandWithTimeout( - ["npm", "install", "--omit=dev", "--omit=peer", "--silent", "--ignore-scripts"], + // Plugins install into isolated directories, so omitting peer deps can strip + // runtime requirements that npm would otherwise materialize for the package. + ["npm", "install", "--omit=dev", "--silent", "--ignore-scripts"], { timeoutMs: Math.max(params.timeoutMs, 300_000), cwd: stageDir, diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index 031d75b31b7..d2ecfab18be 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -1472,6 +1472,30 @@ describe("loadOpenClawPlugins", () => { ).toBe(true); }); + it("dedupes the open allowlist warning for repeated loads of the same plugin set", () => { + useNoBundledPlugins(); + clearPluginLoaderCache(); + const plugin = writePlugin({ + id: "warn-open-allow-once", + body: `module.exports = { id: "warn-open-allow-once", register() {} };`, + }); + const warnings: string[] = []; + const options = { + cache: false, + logger: createWarningLogger(warnings), + config: { + plugins: { + load: { paths: [plugin.file] }, + }, + }, + } as const; + + loadOpenClawPlugins(options); + loadOpenClawPlugins(options); + + expect(warnings.filter((msg) => msg.includes("plugins.allow is empty"))).toHaveLength(1); + }); + it("does not auto-load workspace-discovered plugins unless explicitly trusted", () => { useNoBundledPlugins(); const workspaceDir = makeTempDir(); diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index 40983b43347..75882a5105b 100644 --- a/src/plugins/loader.ts +++ b/src/plugins/loader.ts @@ -51,9 +51,11 @@ export type PluginLoadOptions = { const MAX_PLUGIN_REGISTRY_CACHE_ENTRIES = 32; const registryCache = new Map(); +const openAllowlistWarningCache = new Set(); export function clearPluginLoaderCache(): void { registryCache.clear(); + openAllowlistWarningCache.clear(); } const defaultLogger = () => createSubsystemLogger("plugins"); @@ -455,6 +457,7 @@ function warnWhenAllowlistIsOpen(params: { logger: PluginLogger; pluginsEnabled: boolean; allow: string[]; + warningCacheKey: string; discoverablePlugins: Array<{ id: string; source: string; origin: PluginRecord["origin"] }>; }) { if (!params.pluginsEnabled) { @@ -467,11 +470,15 @@ function warnWhenAllowlistIsOpen(params: { if (nonBundled.length === 0) { return; } + if (openAllowlistWarningCache.has(params.warningCacheKey)) { + return; + } const preview = nonBundled .slice(0, 6) .map((entry) => `${entry.id} (${entry.source})`) .join(", "); const extra = nonBundled.length > 6 ? ` (+${nonBundled.length - 6} more)` : ""; + openAllowlistWarningCache.add(params.warningCacheKey); params.logger.warn( `[plugins] plugins.allow is empty; discovered non-bundled plugins may auto-load: ${preview}${extra}. Set plugins.allow to explicit trusted ids.`, ); @@ -598,6 +605,7 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi logger, pluginsEnabled: normalized.enabled, allow: normalized.allow, + warningCacheKey: cacheKey, discoverablePlugins: manifestRegistry.plugins.map((plugin) => ({ id: plugin.id, source: plugin.source,