From f3971571fe2c12267594eac732b40177ad726ad1 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 19 Mar 2026 16:04:19 -0700 Subject: [PATCH] fix(plugins): fail strict bootstrap on plugin load errors --- src/cli/plugin-registry.test.ts | 8 ++++++-- src/cli/plugin-registry.ts | 1 + src/plugins/loader.test.ts | 29 +++++++++++++++++++++++++++++ src/plugins/loader.ts | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/src/cli/plugin-registry.test.ts b/src/cli/plugin-registry.test.ts index 336c720dfdb..30714b8a023 100644 --- a/src/cli/plugin-registry.test.ts +++ b/src/cli/plugin-registry.test.ts @@ -60,6 +60,7 @@ describe("ensurePluginRegistryLoaded", () => { expect(mocks.loadOpenClawPlugins).toHaveBeenCalledWith( expect.objectContaining({ onlyPluginIds: [], + throwOnLoadError: true, }), ); }); @@ -85,11 +86,14 @@ describe("ensurePluginRegistryLoaded", () => { expect(mocks.loadOpenClawPlugins).toHaveBeenCalledTimes(2); expect(mocks.loadOpenClawPlugins).toHaveBeenNthCalledWith( 1, - expect.objectContaining({ onlyPluginIds: [] }), + expect.objectContaining({ onlyPluginIds: [], throwOnLoadError: true }), ); expect(mocks.loadOpenClawPlugins).toHaveBeenNthCalledWith( 2, - expect.objectContaining({ onlyPluginIds: ["telegram", "slack"] }), + expect.objectContaining({ + onlyPluginIds: ["telegram", "slack"], + throwOnLoadError: true, + }), ); }); }); diff --git a/src/cli/plugin-registry.ts b/src/cli/plugin-registry.ts index bff91129204..4656a1d3756 100644 --- a/src/cli/plugin-registry.ts +++ b/src/cli/plugin-registry.ts @@ -55,6 +55,7 @@ export function ensurePluginRegistryLoaded(options?: { scope?: PluginRegistrySco config, workspaceDir, logger, + throwOnLoadError: true, ...(scope === "configured-channels" ? { onlyPluginIds: resolveConfiguredChannelPluginIds({ diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index c99ccbf41f0..489ab3ce294 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -1625,6 +1625,35 @@ module.exports = { id: "skipped-scoped-only", register() { throw new Error("skip expect(registry.diagnostics.some((d) => d.level === "error")).toBe(true); }); + it("throws when strict plugin loading sees plugin errors", () => { + useNoBundledPlugins(); + const plugin = writePlugin({ + id: "configurable", + filename: "configurable.cjs", + body: `module.exports = { id: "configurable", register() {} };`, + }); + + expect(() => + loadOpenClawPlugins({ + cache: false, + throwOnLoadError: true, + config: { + plugins: { + enabled: true, + load: { paths: [plugin.file] }, + allow: ["configurable"], + entries: { + configurable: { + enabled: true, + config: "nope" as unknown as Record, + }, + }, + }, + }, + }), + ).toThrow("plugin load failed: configurable: invalid config: : must be object"); + }); + it("fails when plugin export id mismatches manifest id", () => { useNoBundledPlugins(); const plugin = writePlugin({ diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index 68ba5b8403a..03a1b0810ff 100644 --- a/src/plugins/loader.ts +++ b/src/plugins/loader.ts @@ -71,8 +71,25 @@ export type PluginLoadOptions = { */ preferSetupRuntimeForChannelPlugins?: boolean; activate?: boolean; + throwOnLoadError?: boolean; }; +export class PluginLoadFailureError extends Error { + readonly pluginIds: string[]; + readonly registry: PluginRegistry; + + constructor(registry: PluginRegistry) { + const failedPlugins = registry.plugins.filter((entry) => entry.status === "error"); + const summary = failedPlugins + .map((entry) => `${entry.id}: ${entry.error ?? "unknown plugin load error"}`) + .join("; "); + super(`plugin load failed: ${summary}`); + this.name = "PluginLoadFailureError"; + this.pluginIds = failedPlugins.map((entry) => entry.id); + this.registry = registry; + } +} + const MAX_PLUGIN_REGISTRY_CACHE_ENTRIES = 128; const registryCache = new Map(); const openAllowlistWarningCache = new Set(); @@ -413,6 +430,19 @@ function pushDiagnostics(diagnostics: PluginDiagnostic[], append: PluginDiagnost diagnostics.push(...append); } +function maybeThrowOnPluginLoadError( + registry: PluginRegistry, + throwOnLoadError: boolean | undefined, +): void { + if (!throwOnLoadError) { + return; + } + if (!registry.plugins.some((entry) => entry.status === "error")) { + return; + } + throw new PluginLoadFailureError(registry); +} + type PathMatcher = { exact: Set; dirs: string[]; @@ -1253,6 +1283,8 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi env, }); + maybeThrowOnPluginLoadError(registry, options.throwOnLoadError); + if (cacheEnabled) { setCachedPluginRegistry(cacheKey, registry); }