From dd472deab47d480e114f36fc8784d50cdae1d088 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Jul 2026 06:54:14 -0400 Subject: [PATCH] fix(plugins): preserve settings for compatibility IDs (#114521) Honor canonical plugin identity across allowlists, denylists, and plugin mutations. Preserve and deterministically deep-merge alias configuration, and explicitly persist canonical entries in CLI enable and disable commands. Closes #114320 Co-authored-by: kevin2966n <147227108+kevin2966n@users.noreply.github.com> --- src/cli/plugins-cli.policy.test.ts | 30 +++++++++ src/cli/plugins-cli.runtime.ts | 10 +++ src/cli/plugins-config.test.ts | 98 ++++++++++++++++++++++++++++++ src/plugins/enable.test.ts | 41 +++++++++++++ src/plugins/enable.ts | 16 ++--- src/plugins/toggle-config.ts | 35 ++++++++--- 6 files changed, 213 insertions(+), 17 deletions(-) diff --git a/src/cli/plugins-cli.policy.test.ts b/src/cli/plugins-cli.policy.test.ts index 7d9a2b38c2c8..87fc744612a3 100644 --- a/src/cli/plugins-cli.policy.test.ts +++ b/src/cli/plugins-cli.policy.test.ts @@ -5,6 +5,7 @@ import { buildPluginRegistrySnapshotReport, enablePluginInConfig, loadConfig, + replaceConfigFile, refreshPluginRegistry, resetPluginsCliTestState, runtimeErrors, @@ -85,6 +86,13 @@ describe("plugins cli policy mutations", () => { expect(enablePluginInConfig).toHaveBeenCalledWith(sourceConfig, "alpha", { updateChannelConfig: false, }); + expect(replaceConfigFile).toHaveBeenCalledWith({ + nextConfig: enabledConfig, + baseHash: "mock", + writeOptions: { + explicitSetPaths: [["plugins", "entries", "alpha"]], + }, + }); expect(writeConfigFile).toHaveBeenCalledWith(enabledConfig); expect(refreshPluginRegistry).toHaveBeenCalledWith({ config: enabledConfig, @@ -162,6 +170,13 @@ describe("plugins cli policy mutations", () => { const nextConfig = requireFirstWrittenConfig(); const entries = requirePluginEntries(nextConfig); expect(entries.alpha).toEqual({ enabled: false }); + expect(replaceConfigFile).toHaveBeenCalledWith({ + nextConfig, + baseHash: "mock", + writeOptions: { + explicitSetPaths: [["plugins", "entries", "alpha"]], + }, + }); expect(refreshPluginRegistry).toHaveBeenCalledWith({ config: nextConfig, installRecords: {}, @@ -185,6 +200,7 @@ describe("plugins cli policy mutations", () => { enablePluginInConfig.mockReturnValue({ config: enabledConfig, enabled: true, + pluginId, }); mockPluginRegistry([pluginId]); @@ -193,6 +209,13 @@ describe("plugins cli policy mutations", () => { expect(enablePluginInConfig).toHaveBeenCalledWith(sourceConfig, pluginId, { updateChannelConfig: false, }); + expect(replaceConfigFile).toHaveBeenCalledWith({ + nextConfig: enabledConfig, + baseHash: "mock", + writeOptions: { + explicitSetPaths: [["plugins", "entries", pluginId]], + }, + }); expect(writeConfigFile).toHaveBeenCalledWith(enabledConfig); }, ); @@ -215,6 +238,13 @@ describe("plugins cli policy mutations", () => { const entries = requirePluginEntries(nextConfig); expect(entries[pluginId]).toEqual({ enabled: false }); expect(entries[alias]).toBeUndefined(); + expect(replaceConfigFile).toHaveBeenCalledWith({ + nextConfig, + baseHash: "mock", + writeOptions: { + explicitSetPaths: [["plugins", "entries", pluginId]], + }, + }); }, ); diff --git a/src/cli/plugins-cli.runtime.ts b/src/cli/plugins-cli.runtime.ts index 5c9360d42c19..7ffe0e78aae3 100644 --- a/src/cli/plugins-cli.runtime.ts +++ b/src/cli/plugins-cli.runtime.ts @@ -232,6 +232,11 @@ async function runPluginsEnableCommandUnlocked(idInput: string): Promise { await replaceConfigFile({ nextConfig: next, ...(snapshot.hash !== undefined ? { baseHash: snapshot.hash } : {}), + // Source/runtime projection must retain the explicitly merged canonical + // entry; otherwise compatibility-only nested settings are silently lost. + writeOptions: { + explicitSetPaths: [["plugins", "entries", enableResult.pluginId]], + }, }); await refreshPluginRegistryAfterConfigMutation({ config: next, @@ -276,6 +281,11 @@ async function runPluginsDisableCommandUnlocked(idInput: string): Promise await replaceConfigFile({ nextConfig: next, ...(snapshot.hash !== undefined ? { baseHash: snapshot.hash } : {}), + // `id` was normalized before discovery; persist that same canonical entry + // so alias invocations cannot lose settings during source projection. + writeOptions: { + explicitSetPaths: [["plugins", "entries", id]], + }, }); await refreshPluginRegistryAfterConfigMutation({ config: next, diff --git a/src/cli/plugins-config.test.ts b/src/cli/plugins-config.test.ts index 8338a0c651ff..c2e8b8c318da 100644 --- a/src/cli/plugins-config.test.ts +++ b/src/cli/plugins-config.test.ts @@ -31,6 +31,104 @@ describe("setPluginEnabledInConfig", () => { }); }); + it.each([ + { action: "enables", enabled: true }, + { action: "disables", enabled: false }, + ])("$action one canonical compatibility entry without losing its config", ({ enabled }) => { + const config = { + plugins: { + allow: [" GOOGLE-GEMINI-CLI "], + entries: { + "GOOGLE-GEMINI-CLI": { + enabled: !enabled, + custom: "preserved", + config: { region: "us" }, + }, + }, + }, + } as OpenClawConfig; + + const next = setPluginEnabledInConfig(config, "google", enabled); + + expect(next.plugins?.allow).toEqual(["google"]); + expect(next.plugins?.entries).toEqual({ + google: { + enabled, + custom: "preserved", + config: { region: "us" }, + }, + }); + }); + + it.each([ + { + name: "canonical entry last", + entries: { + "GOOGLE-GEMINI-CLI": { + config: { + region: "us", + nested: { legacy: true, shared: "legacy" }, + }, + custom: "legacy", + enabled: true, + }, + google: { + config: { + model: "gemini", + nested: { canonical: true, shared: "canonical" }, + }, + custom: "canonical", + enabled: false, + }, + }, + }, + { + name: "canonical entry first", + entries: { + google: { + config: { + model: "gemini", + nested: { canonical: true, shared: "canonical" }, + }, + custom: "canonical", + enabled: false, + }, + "GOOGLE-GEMINI-CLI": { + config: { + region: "us", + nested: { legacy: true, shared: "legacy" }, + }, + custom: "legacy", + enabled: true, + }, + }, + }, + ])("deep-merges compatibility settings with $name", ({ entries }) => { + const config = { + plugins: { + entries, + }, + } as OpenClawConfig; + + const next = setPluginEnabledInConfig(config, "google", true); + + expect(next.plugins?.entries).toEqual({ + google: { + config: { + model: "gemini", + nested: { + canonical: true, + legacy: true, + shared: "canonical", + }, + region: "us", + }, + custom: "canonical", + enabled: true, + }, + }); + }); + it("keeps built-in channel and plugin entry flags in sync", () => { const config = { channels: { diff --git a/src/plugins/enable.test.ts b/src/plugins/enable.test.ts index 9091108646c3..5054bb14f491 100644 --- a/src/plugins/enable.test.ts +++ b/src/plugins/enable.test.ts @@ -77,6 +77,32 @@ describe("enablePluginInConfig", () => { expectEnabledAllowlist(result, ["google"]); }, }, + { + name: "enables a canonical plugin allowed through a mixed-case compatibility id", + cfg: { + plugins: { + allow: [" GOOGLE-GEMINI-CLI "], + }, + } as OpenClawConfig, + pluginId: "google", + expectedEnabled: true, + assert: (result: ReturnType) => { + expect(result.pluginId).toBe("google"); + expect(result.config.plugins?.entries?.google?.enabled).toBe(true); + expectEnabledAllowlist(result, ["google"]); + }, + }, + { + name: "canonicalizes a mixed-case compatibility target before enabling it", + cfg: {} as OpenClawConfig, + pluginId: " GOOGLE-GEMINI-CLI ", + expectedEnabled: true, + assert: (result: ReturnType) => { + expect(result.pluginId).toBe("google"); + expect(result.config.plugins?.entries?.google?.enabled).toBe(true); + expect(result.config.plugins?.entries?.[" GOOGLE-GEMINI-CLI "]).toBeUndefined(); + }, + }, { name: "refuses enable when plugin is denylisted", cfg: { @@ -90,6 +116,21 @@ describe("enablePluginInConfig", () => { expect(result.reason).toBe("blocked by denylist"); }, }, + { + name: "refuses a canonical plugin denied through a mixed-case compatibility id", + cfg: { + plugins: { + deny: [" GOOGLE-GEMINI-CLI "], + }, + } as OpenClawConfig, + pluginId: "google", + expectedEnabled: false, + assert: (result: ReturnType) => { + expect(result.pluginId).toBe("google"); + expect(result.reason).toBe("blocked by denylist"); + expect(result.config.plugins?.entries?.google).toBeUndefined(); + }, + }, { name: "writes built-in channels to channels..enabled and plugins.entries", cfg: {} as OpenClawConfig, diff --git a/src/plugins/enable.ts b/src/plugins/enable.ts index 9b09d8444769..206f66f05a61 100644 --- a/src/plugins/enable.ts +++ b/src/plugins/enable.ts @@ -2,6 +2,7 @@ import { normalizeChatChannelId } from "../channels/ids.js"; import { ensurePluginAllowlisted } from "../config/plugins-allowlist.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { normalizePluginId, normalizePluginsConfig } from "./config-state.js"; import { setPluginEnabledInConfig } from "./toggle-config.js"; type PluginEnableOptions = { @@ -23,20 +24,15 @@ export function enablePluginInConfig( options: PluginEnableOptions = {}, ): PluginEnableResult { const builtInChannelId = normalizeChatChannelId(pluginId); - const resolvedId = builtInChannelId ?? pluginId; - if (cfg.plugins?.enabled === false) { + const resolvedId = normalizePluginId(builtInChannelId ?? pluginId); + const plugins = normalizePluginsConfig(cfg.plugins); + if (!plugins.enabled) { return { config: cfg, enabled: false, pluginId: resolvedId, reason: "plugins disabled" }; } - if (cfg.plugins?.deny?.includes(pluginId) || cfg.plugins?.deny?.includes(resolvedId)) { + if (plugins.deny.includes(resolvedId)) { return { config: cfg, enabled: false, pluginId: resolvedId, reason: "blocked by denylist" }; } - const allow = cfg.plugins?.allow; - if ( - Array.isArray(allow) && - allow.length > 0 && - !allow.includes(pluginId) && - !allow.includes(resolvedId) - ) { + if (plugins.allow.length > 0 && !plugins.allow.includes(resolvedId)) { return { config: cfg, enabled: false, pluginId: resolvedId, reason: "blocked by allowlist" }; } return { diff --git a/src/plugins/toggle-config.ts b/src/plugins/toggle-config.ts index 78e2b4fed0a6..0fa55e942478 100644 --- a/src/plugins/toggle-config.ts +++ b/src/plugins/toggle-config.ts @@ -1,6 +1,9 @@ // Toggles plugin enablement config for channels and agents. import { normalizeChatChannelId } from "../channels/ids.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; +import type { PluginEntryConfig } from "../config/types.plugins.js"; +import { mergeDeep } from "../infra/deep-merge.js"; +import { normalizePluginId, normalizePluginTargetConfig } from "./config-state.js"; /** Returns config with a plugin enabled/disabled and optional built-in channel state synced. */ export function setPluginEnabledInConfig( @@ -10,16 +13,34 @@ export function setPluginEnabledInConfig( options: { updateChannelConfig?: boolean } = {}, ): OpenClawConfig { const builtInChannelId = normalizeChatChannelId(pluginId); - const resolvedId = builtInChannelId ?? pluginId; + const resolvedId = normalizePluginId(builtInChannelId ?? pluginId); + const normalizedConfig = normalizePluginTargetConfig(config, resolvedId); + let existingEntry: PluginEntryConfig = {}; + // Fold aliases first and the canonical entry last so duplicate config keeps + // every nested setting while canonical values win independent of file order. + const existingEntries = Object.entries(config.plugins?.entries ?? {}) + .filter(([entryId]) => normalizePluginId(entryId) === resolvedId) + .toSorted(([leftId], [rightId]) => { + if (leftId === resolvedId) { + return rightId === resolvedId ? 0 : 1; + } + if (rightId === resolvedId) { + return -1; + } + return leftId.localeCompare(rightId, "en"); + }); + for (const [, entry] of existingEntries) { + existingEntry = mergeDeep(existingEntry, entry) as PluginEntryConfig; + } const next: OpenClawConfig = { - ...config, + ...normalizedConfig, plugins: { - ...config.plugins, + ...normalizedConfig.plugins, entries: { - ...config.plugins?.entries, + ...normalizedConfig.plugins?.entries, [resolvedId]: { - ...(config.plugins?.entries?.[resolvedId] as object | undefined), + ...existingEntry, enabled, }, }, @@ -30,7 +51,7 @@ export function setPluginEnabledInConfig( return next; } - const channels = config.channels as Record | undefined; + const channels = normalizedConfig.channels as Record | undefined; const existing = channels?.[builtInChannelId]; const existingRecord = existing && typeof existing === "object" && !Array.isArray(existing) @@ -40,7 +61,7 @@ export function setPluginEnabledInConfig( return { ...next, channels: { - ...config.channels, + ...normalizedConfig.channels, [builtInChannelId]: { ...existingRecord, enabled,