mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-02 09:11:34 +00:00
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>
This commit is contained in:
committed by
GitHub
parent
521f45592e
commit
dd472deab4
@@ -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]],
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -232,6 +232,11 @@ async function runPluginsEnableCommandUnlocked(idInput: string): Promise<void> {
|
||||
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<void>
|
||||
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,
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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<typeof enablePluginInConfig>) => {
|
||||
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<typeof enablePluginInConfig>) => {
|
||||
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<typeof enablePluginInConfig>) => {
|
||||
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.<id>.enabled and plugins.entries",
|
||||
cfg: {} as OpenClawConfig,
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<string, unknown> | undefined;
|
||||
const channels = normalizedConfig.channels as Record<string, unknown> | 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,
|
||||
|
||||
Reference in New Issue
Block a user