mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-18 13:30:48 +00:00
fix(onboarding): use scoped plugin snapshots to prevent OOM on low-memory hosts (#46763)
* fix(onboarding): use scoped plugin snapshots to prevent OOM on low-memory hosts Onboarding and channel-add flows previously loaded the full plugin registry, which caused OOM crashes on memory-constrained hosts. This patch introduces scoped, non-activating plugin registry snapshots that load only the selected channel plugin without replacing the running gateway's global state. Key changes: - Add onlyPluginIds and activate options to loadOpenClawPlugins for scoped loads - Add suppressGlobalCommands to plugin registry to avoid leaking commands - Replace full registry reloads in onboarding with per-channel scoped snapshots - Validate command definitions in snapshot loads without writing global registry - Preload configured external plugins via scoped discovery during onboarding Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): add return type annotation to hoisted mock to resolve TS2322 * fix(plugins): enforce cache:false invariant for non-activating snapshot loads * Channels: preserve lazy scoped snapshot import after rebase * Onboarding: scope channel snapshots by plugin id * Catalog: trust manifest ids for channel plugin mapping * Onboarding: preserve scoped setup channel loading * Onboarding: restore built-in adapter fallback --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
@@ -111,6 +111,29 @@ export type CommandRegistrationResult = {
|
||||
error?: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Validate a plugin command definition without registering it.
|
||||
* Returns an error message if invalid, or null if valid.
|
||||
* Shared by both the global registration path and snapshot (non-activating) loads.
|
||||
*/
|
||||
export function validatePluginCommandDefinition(
|
||||
command: OpenClawPluginCommandDefinition,
|
||||
): string | null {
|
||||
if (typeof command.handler !== "function") {
|
||||
return "Command handler must be a function";
|
||||
}
|
||||
if (typeof command.name !== "string") {
|
||||
return "Command name must be a string";
|
||||
}
|
||||
if (typeof command.description !== "string") {
|
||||
return "Command description must be a string";
|
||||
}
|
||||
if (!command.description.trim()) {
|
||||
return "Command description cannot be empty";
|
||||
}
|
||||
return validateCommandName(command.name.trim());
|
||||
}
|
||||
|
||||
/**
|
||||
* Register a plugin command.
|
||||
* Returns an error if the command name is invalid or reserved.
|
||||
@@ -125,28 +148,13 @@ export function registerPluginCommand(
|
||||
return { ok: false, error: "Cannot register commands while processing is in progress" };
|
||||
}
|
||||
|
||||
// Validate handler is a function
|
||||
if (typeof command.handler !== "function") {
|
||||
return { ok: false, error: "Command handler must be a function" };
|
||||
}
|
||||
|
||||
if (typeof command.name !== "string") {
|
||||
return { ok: false, error: "Command name must be a string" };
|
||||
}
|
||||
if (typeof command.description !== "string") {
|
||||
return { ok: false, error: "Command description must be a string" };
|
||||
const definitionError = validatePluginCommandDefinition(command);
|
||||
if (definitionError) {
|
||||
return { ok: false, error: definitionError };
|
||||
}
|
||||
|
||||
const name = command.name.trim();
|
||||
const description = command.description.trim();
|
||||
if (!description) {
|
||||
return { ok: false, error: "Command description cannot be empty" };
|
||||
}
|
||||
|
||||
const validationError = validateCommandName(name);
|
||||
if (validationError) {
|
||||
return { ok: false, error: validationError };
|
||||
}
|
||||
|
||||
const key = `/${name.toLowerCase()}`;
|
||||
|
||||
|
||||
@@ -14,15 +14,19 @@ async function importFreshPluginTestModules() {
|
||||
vi.unmock("./hooks.js");
|
||||
vi.unmock("./loader.js");
|
||||
vi.unmock("jiti");
|
||||
const [loader, hookRunnerGlobal, hooks] = await Promise.all([
|
||||
const [loader, hookRunnerGlobal, hooks, runtime, registry] = await Promise.all([
|
||||
import("./loader.js"),
|
||||
import("./hook-runner-global.js"),
|
||||
import("./hooks.js"),
|
||||
import("./runtime.js"),
|
||||
import("./registry.js"),
|
||||
]);
|
||||
return {
|
||||
...loader,
|
||||
...hookRunnerGlobal,
|
||||
...hooks,
|
||||
...runtime,
|
||||
...registry,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -30,9 +34,13 @@ const {
|
||||
__testing,
|
||||
clearPluginLoaderCache,
|
||||
createHookRunner,
|
||||
createEmptyPluginRegistry,
|
||||
getActivePluginRegistry,
|
||||
getActivePluginRegistryKey,
|
||||
getGlobalHookRunner,
|
||||
loadOpenClawPlugins,
|
||||
resetGlobalHookRunner,
|
||||
setActivePluginRegistry,
|
||||
} = await importFreshPluginTestModules();
|
||||
|
||||
type TempPlugin = { dir: string; file: string; id: string };
|
||||
@@ -580,6 +588,112 @@ describe("loadOpenClawPlugins", () => {
|
||||
expect(Object.keys(registry.gatewayHandlers)).toContain("allowed.ping");
|
||||
});
|
||||
|
||||
it("limits imports to the requested plugin ids", () => {
|
||||
useNoBundledPlugins();
|
||||
const allowed = writePlugin({
|
||||
id: "allowed",
|
||||
filename: "allowed.cjs",
|
||||
body: `module.exports = { id: "allowed", register() {} };`,
|
||||
});
|
||||
const skippedMarker = path.join(makeTempDir(), "skipped-loaded.txt");
|
||||
const skipped = writePlugin({
|
||||
id: "skipped",
|
||||
filename: "skipped.cjs",
|
||||
body: `require("node:fs").writeFileSync(${JSON.stringify(skippedMarker)}, "loaded", "utf-8");
|
||||
module.exports = { id: "skipped", register() { throw new Error("skipped plugin should not load"); } };`,
|
||||
});
|
||||
|
||||
const registry = loadOpenClawPlugins({
|
||||
cache: false,
|
||||
config: {
|
||||
plugins: {
|
||||
load: { paths: [allowed.file, skipped.file] },
|
||||
allow: ["allowed", "skipped"],
|
||||
},
|
||||
},
|
||||
onlyPluginIds: ["allowed"],
|
||||
});
|
||||
|
||||
expect(registry.plugins.map((entry) => entry.id)).toEqual(["allowed"]);
|
||||
expect(fs.existsSync(skippedMarker)).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps scoped plugin loads in a separate cache entry", () => {
|
||||
useNoBundledPlugins();
|
||||
const allowed = writePlugin({
|
||||
id: "allowed",
|
||||
filename: "allowed.cjs",
|
||||
body: `module.exports = { id: "allowed", register() {} };`,
|
||||
});
|
||||
const extra = writePlugin({
|
||||
id: "extra",
|
||||
filename: "extra.cjs",
|
||||
body: `module.exports = { id: "extra", register() {} };`,
|
||||
});
|
||||
const options = {
|
||||
config: {
|
||||
plugins: {
|
||||
load: { paths: [allowed.file, extra.file] },
|
||||
allow: ["allowed", "extra"],
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const full = loadOpenClawPlugins(options);
|
||||
const scoped = loadOpenClawPlugins({
|
||||
...options,
|
||||
onlyPluginIds: ["allowed"],
|
||||
});
|
||||
const scopedAgain = loadOpenClawPlugins({
|
||||
...options,
|
||||
onlyPluginIds: ["allowed"],
|
||||
});
|
||||
|
||||
expect(full.plugins.map((entry) => entry.id).toSorted()).toEqual(["allowed", "extra"]);
|
||||
expect(scoped).not.toBe(full);
|
||||
expect(scoped.plugins.map((entry) => entry.id)).toEqual(["allowed"]);
|
||||
expect(scopedAgain).toBe(scoped);
|
||||
});
|
||||
|
||||
it("can load a scoped registry without replacing the active global registry", () => {
|
||||
useNoBundledPlugins();
|
||||
const plugin = writePlugin({
|
||||
id: "allowed",
|
||||
filename: "allowed.cjs",
|
||||
body: `module.exports = { id: "allowed", register() {} };`,
|
||||
});
|
||||
const previousRegistry = createEmptyPluginRegistry();
|
||||
setActivePluginRegistry(previousRegistry, "existing-registry");
|
||||
resetGlobalHookRunner();
|
||||
|
||||
const scoped = loadOpenClawPlugins({
|
||||
cache: false,
|
||||
activate: false,
|
||||
workspaceDir: plugin.dir,
|
||||
config: {
|
||||
plugins: {
|
||||
load: { paths: [plugin.file] },
|
||||
allow: ["allowed"],
|
||||
},
|
||||
},
|
||||
onlyPluginIds: ["allowed"],
|
||||
});
|
||||
|
||||
expect(scoped.plugins.map((entry) => entry.id)).toEqual(["allowed"]);
|
||||
expect(getActivePluginRegistry()).toBe(previousRegistry);
|
||||
expect(getActivePluginRegistryKey()).toBe("existing-registry");
|
||||
expect(getGlobalHookRunner()).toBeNull();
|
||||
});
|
||||
|
||||
it("throws when activate:false is used without cache:false", () => {
|
||||
expect(() => loadOpenClawPlugins({ activate: false })).toThrow(
|
||||
"activate:false requires cache:false",
|
||||
);
|
||||
expect(() => loadOpenClawPlugins({ activate: false, cache: true })).toThrow(
|
||||
"activate:false requires cache:false",
|
||||
);
|
||||
});
|
||||
|
||||
it("re-initializes global hook runner when serving registry from cache", () => {
|
||||
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = "/nonexistent/bundled/plugins";
|
||||
const plugin = writePlugin({
|
||||
|
||||
@@ -50,6 +50,8 @@ export type PluginLoadOptions = {
|
||||
runtimeOptions?: CreatePluginRuntimeOptions;
|
||||
cache?: boolean;
|
||||
mode?: "full" | "validate";
|
||||
onlyPluginIds?: string[];
|
||||
activate?: boolean;
|
||||
};
|
||||
|
||||
const MAX_PLUGIN_REGISTRY_CACHE_ENTRIES = 32;
|
||||
@@ -241,6 +243,7 @@ function buildCacheKey(params: {
|
||||
plugins: NormalizedPluginsConfig;
|
||||
installs?: Record<string, PluginInstallRecord>;
|
||||
env: NodeJS.ProcessEnv;
|
||||
onlyPluginIds?: string[];
|
||||
}): string {
|
||||
const { roots, loadPaths } = resolvePluginCacheInputs({
|
||||
workspaceDir: params.workspaceDir,
|
||||
@@ -263,11 +266,20 @@ function buildCacheKey(params: {
|
||||
},
|
||||
]),
|
||||
);
|
||||
const scopeKey = JSON.stringify(params.onlyPluginIds ?? []);
|
||||
return `${roots.workspace ?? ""}::${roots.global ?? ""}::${roots.stock ?? ""}::${JSON.stringify({
|
||||
...params.plugins,
|
||||
installs,
|
||||
loadPaths,
|
||||
})}`;
|
||||
})}::${scopeKey}`;
|
||||
}
|
||||
|
||||
function normalizeScopedPluginIds(ids?: string[]): string[] | undefined {
|
||||
if (!ids) {
|
||||
return undefined;
|
||||
}
|
||||
const normalized = Array.from(new Set(ids.map((id) => id.trim()).filter(Boolean))).toSorted();
|
||||
return normalized.length > 0 ? normalized : undefined;
|
||||
}
|
||||
|
||||
function validatePluginConfig(params: {
|
||||
@@ -640,6 +652,13 @@ function activatePluginRegistry(registry: PluginRegistry, cacheKey: string): voi
|
||||
}
|
||||
|
||||
export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegistry {
|
||||
// Snapshot (non-activating) loads must disable the cache to avoid storing a registry
|
||||
// whose commands were never globally registered.
|
||||
if (options.activate === false && options.cache !== false) {
|
||||
throw new Error(
|
||||
"loadOpenClawPlugins: activate:false requires cache:false to prevent command registry divergence",
|
||||
);
|
||||
}
|
||||
const env = options.env ?? process.env;
|
||||
// Test env: default-disable plugins unless explicitly configured.
|
||||
// This keeps unit/gateway suites fast and avoids loading heavyweight plugin deps by accident.
|
||||
@@ -647,24 +666,37 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
|
||||
const logger = options.logger ?? defaultLogger();
|
||||
const validateOnly = options.mode === "validate";
|
||||
const normalized = normalizePluginsConfig(cfg.plugins);
|
||||
const onlyPluginIds = normalizeScopedPluginIds(options.onlyPluginIds);
|
||||
const onlyPluginIdSet = onlyPluginIds ? new Set(onlyPluginIds) : null;
|
||||
const shouldActivate = options.activate !== false;
|
||||
// NOTE: `activate` is intentionally excluded from the cache key. All non-activating
|
||||
// (snapshot) callers pass `cache: false` via loadOnboardingPluginRegistry(), so they
|
||||
// never read from or write to the cache. Including `activate` here would be misleading
|
||||
// — it would imply mixed-activate caching is supported, when in practice it is not.
|
||||
const cacheKey = buildCacheKey({
|
||||
workspaceDir: options.workspaceDir,
|
||||
plugins: normalized,
|
||||
installs: cfg.plugins?.installs,
|
||||
env,
|
||||
onlyPluginIds,
|
||||
});
|
||||
const cacheEnabled = options.cache !== false;
|
||||
if (cacheEnabled) {
|
||||
const cached = getCachedPluginRegistry(cacheKey);
|
||||
if (cached) {
|
||||
activatePluginRegistry(cached, cacheKey);
|
||||
if (shouldActivate) {
|
||||
activatePluginRegistry(cached, cacheKey);
|
||||
}
|
||||
return cached;
|
||||
}
|
||||
}
|
||||
|
||||
// Clear previously registered plugin commands before reloading
|
||||
clearPluginCommands();
|
||||
clearPluginInteractiveHandlers();
|
||||
// Clear previously registered plugin commands before reloading.
|
||||
// Skip for non-activating (snapshot) loads to avoid wiping commands from other plugins.
|
||||
if (shouldActivate) {
|
||||
clearPluginCommands();
|
||||
clearPluginInteractiveHandlers();
|
||||
}
|
||||
|
||||
// Lazily initialize the runtime so startup paths that discover/skip plugins do
|
||||
// not eagerly load every channel runtime dependency.
|
||||
@@ -703,6 +735,7 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
|
||||
logger,
|
||||
runtime,
|
||||
coreGatewayHandlers: options.coreGatewayHandlers as Record<string, GatewayRequestHandler>,
|
||||
suppressGlobalCommands: !shouldActivate,
|
||||
});
|
||||
|
||||
const discovery = discoverOpenClawPlugins({
|
||||
@@ -725,11 +758,15 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
|
||||
pluginsEnabled: normalized.enabled,
|
||||
allow: normalized.allow,
|
||||
warningCacheKey: cacheKey,
|
||||
discoverablePlugins: manifestRegistry.plugins.map((plugin) => ({
|
||||
id: plugin.id,
|
||||
source: plugin.source,
|
||||
origin: plugin.origin,
|
||||
})),
|
||||
// Keep warning input scoped as well so partial snapshot loads only mention the
|
||||
// plugins that were intentionally requested for this registry.
|
||||
discoverablePlugins: manifestRegistry.plugins
|
||||
.filter((plugin) => !onlyPluginIdSet || onlyPluginIdSet.has(plugin.id))
|
||||
.map((plugin) => ({
|
||||
id: plugin.id,
|
||||
source: plugin.source,
|
||||
origin: plugin.origin,
|
||||
})),
|
||||
});
|
||||
const provenance = buildProvenanceIndex({
|
||||
config: cfg,
|
||||
@@ -786,6 +823,11 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
|
||||
continue;
|
||||
}
|
||||
const pluginId = manifestRecord.id;
|
||||
// Filter again at import time as a final guard. The earlier manifest filter keeps
|
||||
// warnings scoped; this one prevents loading/registering anything outside the scope.
|
||||
if (onlyPluginIdSet && !onlyPluginIdSet.has(pluginId)) {
|
||||
continue;
|
||||
}
|
||||
const existingOrigin = seenIds.get(pluginId);
|
||||
if (existingOrigin) {
|
||||
const record = createPluginRecord({
|
||||
@@ -1059,7 +1101,9 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
|
||||
}
|
||||
}
|
||||
|
||||
if (typeof memorySlot === "string" && !memorySlotMatched) {
|
||||
// Scoped snapshot loads may intentionally omit the configured memory plugin, so only
|
||||
// emit the missing-memory diagnostic for full registry loads.
|
||||
if (!onlyPluginIdSet && typeof memorySlot === "string" && !memorySlotMatched) {
|
||||
registry.diagnostics.push({
|
||||
level: "warn",
|
||||
message: `memory slot plugin not found or not marked as memory: ${memorySlot}`,
|
||||
@@ -1076,7 +1120,9 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
|
||||
if (cacheEnabled) {
|
||||
setCachedPluginRegistry(cacheKey, registry);
|
||||
}
|
||||
activatePluginRegistry(registry, cacheKey);
|
||||
if (shouldActivate) {
|
||||
activatePluginRegistry(registry, cacheKey);
|
||||
}
|
||||
return registry;
|
||||
}
|
||||
|
||||
|
||||
@@ -10,7 +10,7 @@ import type {
|
||||
import { registerInternalHook } from "../hooks/internal-hooks.js";
|
||||
import type { HookEntry } from "../hooks/types.js";
|
||||
import { resolveUserPath } from "../utils.js";
|
||||
import { registerPluginCommand } from "./commands.js";
|
||||
import { registerPluginCommand, validatePluginCommandDefinition } from "./commands.js";
|
||||
import { normalizePluginHttpPath } from "./http-path.js";
|
||||
import { findOverlappingPluginHttpRoute } from "./http-route-overlap.js";
|
||||
import { registerPluginInteractiveHandler } from "./interactive.js";
|
||||
@@ -177,6 +177,9 @@ export type PluginRegistryParams = {
|
||||
logger: PluginLogger;
|
||||
coreGatewayHandlers?: GatewayRequestHandlers;
|
||||
runtime: PluginRuntime;
|
||||
// When true, skip writing to the global plugin command registry during register().
|
||||
// Used by non-activating snapshot loads to avoid leaking commands into the running gateway.
|
||||
suppressGlobalCommands?: boolean;
|
||||
};
|
||||
|
||||
type PluginTypedHookPolicy = {
|
||||
@@ -615,19 +618,37 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Register with the plugin command system (validates name and checks for duplicates)
|
||||
const result = registerPluginCommand(record.id, command, {
|
||||
pluginName: record.name,
|
||||
pluginRoot: record.rootDir,
|
||||
});
|
||||
if (!result.ok) {
|
||||
pushDiagnostic({
|
||||
level: "error",
|
||||
pluginId: record.id,
|
||||
source: record.source,
|
||||
message: `command registration failed: ${result.error}`,
|
||||
// For snapshot (non-activating) loads, record the command locally without touching the
|
||||
// global plugin command registry so running gateway commands stay intact.
|
||||
// We still validate the command definition so diagnostics match the real activation path.
|
||||
// NOTE: cross-plugin duplicate command detection is intentionally skipped here because
|
||||
// snapshot registries are isolated and never write to the global command table. Conflicts
|
||||
// will surface when the plugin is loaded via the normal activation path at gateway startup.
|
||||
if (registryParams.suppressGlobalCommands) {
|
||||
const validationError = validatePluginCommandDefinition(command);
|
||||
if (validationError) {
|
||||
pushDiagnostic({
|
||||
level: "error",
|
||||
pluginId: record.id,
|
||||
source: record.source,
|
||||
message: `command registration failed: ${validationError}`,
|
||||
});
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
const result = registerPluginCommand(record.id, command, {
|
||||
pluginName: record.name,
|
||||
pluginRoot: record.rootDir,
|
||||
});
|
||||
return;
|
||||
if (!result.ok) {
|
||||
pushDiagnostic({
|
||||
level: "error",
|
||||
pluginId: record.id,
|
||||
source: record.source,
|
||||
message: `command registration failed: ${result.error}`,
|
||||
});
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
record.commands.push(name);
|
||||
|
||||
Reference in New Issue
Block a user