From 6b72de77ba372e63077c235e3e9dd1f0362a8059 Mon Sep 17 00:00:00 2001 From: George Zhang Date: Fri, 27 Mar 2026 17:12:52 -0700 Subject: [PATCH] Revert "Plugins: sync channel uninstall cleanup" --- src/cli/plugins-cli-test-helpers.ts | 22 +-- src/cli/plugins-cli.ts | 19 +-- src/cli/plugins-cli.uninstall.test.ts | 88 ---------- src/plugins/uninstall.test.ts | 221 -------------------------- src/plugins/uninstall.ts | 56 +------ 5 files changed, 12 insertions(+), 394 deletions(-) diff --git a/src/cli/plugins-cli-test-helpers.ts b/src/cli/plugins-cli-test-helpers.ts index 6052b9b668f..5d65624f159 100644 --- a/src/cli/plugins-cli-test-helpers.ts +++ b/src/cli/plugins-cli-test-helpers.ts @@ -75,19 +75,14 @@ vi.mock("../plugins/slots.js", () => ({ applyExclusiveSlotSelection: (...args: unknown[]) => applyExclusiveSlotSelection(...args), })); -vi.mock("../plugins/uninstall.js", async () => { - const actual = - await vi.importActual("../plugins/uninstall.js"); - return { - ...actual, - uninstallPlugin: (...args: unknown[]) => uninstallPlugin(...args), - resolveUninstallDirectoryTarget: ({ - installRecord, - }: { - installRecord?: { installPath?: string; sourcePath?: string }; - }) => installRecord?.installPath ?? installRecord?.sourcePath ?? null, - }; -}); +vi.mock("../plugins/uninstall.js", () => ({ + uninstallPlugin: (...args: unknown[]) => uninstallPlugin(...args), + resolveUninstallDirectoryTarget: ({ + installRecord, + }: { + installRecord?: { installPath?: string; sourcePath?: string }; + }) => installRecord?.installPath ?? installRecord?.sourcePath ?? null, +})); vi.mock("../plugins/update.js", () => ({ updateNpmInstalledPlugins: (...args: unknown[]) => updateNpmInstalledPlugins(...args), @@ -209,7 +204,6 @@ export function resetPluginsCliTestState() { allowlist: false, loadPath: false, memorySlot: false, - channelConfig: false, directory: false, }, }); diff --git a/src/cli/plugins-cli.ts b/src/cli/plugins-cli.ts index b446eddd5a4..113c9928a1d 100644 --- a/src/cli/plugins-cli.ts +++ b/src/cli/plugins-cli.ts @@ -17,11 +17,7 @@ import { buildPluginStatusReport, formatPluginCompatibilityNotice, } from "../plugins/status.js"; -import { - resolveUninstallChannelConfigKeys, - resolveUninstallDirectoryTarget, - uninstallPlugin, -} from "../plugins/uninstall.js"; +import { resolveUninstallDirectoryTarget, uninstallPlugin } from "../plugins/uninstall.js"; import { defaultRuntime } from "../runtime.js"; import { formatDocsLink } from "../terminal/links.js"; import { getTerminalTableWidth, renderTable } from "../terminal/table.js"; @@ -630,15 +626,6 @@ export function registerPluginsCli(program: Command) { if (cfg.plugins?.slots?.memory === pluginId) { preview.push(`memory slot (will reset to "memory-core")`); } - const channelIds = plugin?.status === "loaded" ? plugin.channelIds : undefined; - const channels = cfg.channels as Record | undefined; - if (hasInstall && channels) { - for (const key of resolveUninstallChannelConfigKeys(pluginId, { channelIds })) { - if (Object.hasOwn(channels, key)) { - preview.push(`channel config (channels.${key})`); - } - } - } const deleteTarget = !keepFiles ? resolveUninstallDirectoryTarget({ pluginId, @@ -673,7 +660,6 @@ export function registerPluginsCli(program: Command) { const result = await uninstallPlugin({ config: cfg, pluginId, - channelIds, deleteFiles: !keepFiles, extensionsDir, }); @@ -704,9 +690,6 @@ export function registerPluginsCli(program: Command) { if (result.actions.memorySlot) { removed.push("memory slot"); } - if (result.actions.channelConfig) { - removed.push("channel config"); - } if (result.actions.directory) { removed.push("directory"); } diff --git a/src/cli/plugins-cli.uninstall.test.ts b/src/cli/plugins-cli.uninstall.test.ts index 05df42aa0f5..4dfeb02d922 100644 --- a/src/cli/plugins-cli.uninstall.test.ts +++ b/src/cli/plugins-cli.uninstall.test.ts @@ -84,7 +84,6 @@ describe("plugins cli uninstall", () => { allowlist: false, loadPath: false, memorySlot: false, - channelConfig: false, directory: false, }, }); @@ -189,91 +188,4 @@ describe("plugins cli uninstall", () => { }), ); }); - - it("previews and passes loaded channel ids to uninstall", async () => { - loadConfig.mockReturnValue({ - plugins: { - entries: { - "timbot-plugin": { enabled: true }, - }, - installs: { - "timbot-plugin": { - source: "npm", - spec: "timbot-plugin@1.0.0", - installPath: "/tmp/openclaw-state/extensions/timbot-plugin", - }, - }, - }, - channels: { - timbot: { sdkAppId: "123" }, - "timbot-v2": { sdkAppId: "456" }, - }, - } as OpenClawConfig); - buildPluginStatusReport.mockReturnValue({ - plugins: [ - { - id: "timbot-plugin", - name: "Timbot", - status: "loaded", - channelIds: ["timbot", "timbot-v2"], - }, - ], - diagnostics: [], - }); - - await runPluginsCommand(["plugins", "uninstall", "timbot-plugin", "--force"]); - - expect(runtimeLogs.some((line) => line.includes("channel config (channels.timbot)"))).toBe( - true, - ); - expect(runtimeLogs.some((line) => line.includes("channel config (channels.timbot-v2)"))).toBe( - true, - ); - expect(uninstallPlugin).toHaveBeenCalledWith( - expect.objectContaining({ - pluginId: "timbot-plugin", - channelIds: ["timbot", "timbot-v2"], - }), - ); - }); - - it("does not preview unrelated channel config when loaded plugin declares no channels", async () => { - loadConfig.mockReturnValue({ - plugins: { - entries: { - telegram: { enabled: true }, - }, - installs: { - telegram: { - source: "npm", - spec: "telegram@1.0.0", - installPath: "/tmp/openclaw-state/extensions/telegram", - }, - }, - }, - channels: { - telegram: { enabled: true }, - }, - } as OpenClawConfig); - buildPluginStatusReport.mockReturnValue({ - plugins: [ - { - id: "telegram", - name: "Telegram helper", - status: "loaded", - channelIds: [], - }, - ], - diagnostics: [], - }); - - await runPluginsCommand(["plugins", "uninstall", "telegram", "--dry-run"]); - - const previewLine = runtimeLogs.find((line) => line.startsWith("Will remove:")); - if (!previewLine) { - throw new Error("expected uninstall preview line"); - } - expect(previewLine).not.toContain("channel config (channels.telegram)"); - expect(uninstallPlugin).not.toHaveBeenCalled(); - }); }); diff --git a/src/plugins/uninstall.test.ts b/src/plugins/uninstall.test.ts index ca7af737833..9286726afc2 100644 --- a/src/plugins/uninstall.test.ts +++ b/src/plugins/uninstall.test.ts @@ -6,7 +6,6 @@ import type { OpenClawConfig } from "../config/config.js"; import { resolvePluginInstallDir } from "./install.js"; import { removePluginFromConfig, - resolveUninstallChannelConfigKeys, resolveUninstallDirectoryTarget, uninstallPlugin, } from "./uninstall.js"; @@ -102,24 +101,6 @@ async function createPluginDirFixture(baseDir: string, pluginId = "my-plugin") { return pluginDir; } -describe("resolveUninstallChannelConfigKeys", () => { - it("falls back to pluginId when channelIds are unknown", () => { - expect(resolveUninstallChannelConfigKeys("timbot")).toEqual(["timbot"]); - }); - - it("keeps explicit empty channelIds as remove-nothing", () => { - expect(resolveUninstallChannelConfigKeys("telegram", { channelIds: [] })).toEqual([]); - }); - - it("filters shared keys and duplicate channel ids", () => { - expect( - resolveUninstallChannelConfigKeys("bad-plugin", { - channelIds: ["defaults", "discord", "discord", "modelByChannel", "slack"], - }), - ).toEqual(["discord", "slack"]); - }); -}); - describe("removePluginFromConfig", () => { it("removes plugin from entries", () => { const config: OpenClawConfig = { @@ -327,208 +308,6 @@ describe("removePluginFromConfig", () => { expect(result.plugins?.enabled).toBe(true); expect(result.plugins?.deny).toEqual(["denied-plugin"]); }); - - it("removes channel config for installed extension plugin", () => { - const config: OpenClawConfig = { - plugins: { - entries: { - timbot: { enabled: true }, - }, - installs: { - timbot: { source: "npm", spec: "timbot@1.0.0" }, - }, - }, - channels: { - timbot: { sdkAppId: "123", secretKey: "abc" }, - telegram: { enabled: true }, - }, - }; - - const { config: result, actions } = removePluginFromConfig(config, "timbot"); - - expect((result.channels as Record)?.timbot).toBeUndefined(); - expect((result.channels as Record)?.telegram).toEqual({ enabled: true }); - expect(actions.channelConfig).toBe(true); - }); - - it("does not remove channel config for built-in channel without install record", () => { - const config: OpenClawConfig = { - plugins: { - entries: { - telegram: { enabled: true }, - }, - }, - channels: { - telegram: { enabled: true }, - discord: { enabled: true }, - }, - }; - - const { config: result, actions } = removePluginFromConfig(config, "telegram"); - - expect((result.channels as Record)?.telegram).toEqual({ enabled: true }); - expect(actions.channelConfig).toBe(false); - }); - - it("cleans up channels object when removing the only channel config", () => { - const config: OpenClawConfig = { - plugins: { - entries: { - timbot: { enabled: true }, - }, - installs: { - timbot: { source: "npm", spec: "timbot@1.0.0" }, - }, - }, - channels: { - timbot: { sdkAppId: "123" }, - }, - }; - - const { config: result, actions } = removePluginFromConfig(config, "timbot"); - - expect(result.channels).toBeUndefined(); - expect(actions.channelConfig).toBe(true); - }); - - it("does not set channelConfig action when no channel config exists", () => { - const config: OpenClawConfig = { - plugins: { - entries: { - "my-plugin": { enabled: true }, - }, - installs: { - "my-plugin": { source: "npm", spec: "my-plugin@1.0.0" }, - }, - }, - }; - - const { actions } = removePluginFromConfig(config, "my-plugin"); - - expect(actions.channelConfig).toBe(false); - }); - - it("does not remove channel config when plugin has no install record", () => { - const config: OpenClawConfig = { - plugins: { - entries: { - discord: { enabled: true }, - }, - }, - channels: { - discord: { enabled: true, token: "abc" }, - }, - }; - - const { config: result, actions } = removePluginFromConfig(config, "discord"); - - expect((result.channels as Record)?.discord).toEqual({ - enabled: true, - token: "abc", - }); - expect(actions.channelConfig).toBe(false); - }); - - it("removes channel config using explicit channelIds when pluginId differs", () => { - const config: OpenClawConfig = { - plugins: { - entries: { - "timbot-plugin": { enabled: true }, - }, - installs: { - "timbot-plugin": { source: "npm", spec: "timbot-plugin@1.0.0" }, - }, - }, - channels: { - timbot: { sdkAppId: "123" }, - "timbot-v2": { sdkAppId: "456" }, - telegram: { enabled: true }, - }, - }; - - const { config: result, actions } = removePluginFromConfig(config, "timbot-plugin", { - channelIds: ["timbot", "timbot-v2"], - }); - - const ch = result.channels as Record | undefined; - expect(ch?.timbot).toBeUndefined(); - expect(ch?.["timbot-v2"]).toBeUndefined(); - expect(ch?.telegram).toEqual({ enabled: true }); - expect(actions.channelConfig).toBe(true); - }); - - it("preserves shared channel keys", () => { - const config: OpenClawConfig = { - plugins: { - entries: { - timbot: { enabled: true }, - }, - installs: { - timbot: { source: "npm", spec: "timbot@1.0.0" }, - }, - }, - channels: { - defaults: { groupPolicy: "opt-in" }, - modelByChannel: { timbot: "gpt-3.5" } as Record, - timbot: { sdkAppId: "123" }, - } as unknown as OpenClawConfig["channels"], - }; - - const { config: result, actions } = removePluginFromConfig(config, "timbot"); - - const ch = result.channels as Record | undefined; - expect(ch?.timbot).toBeUndefined(); - expect(ch?.defaults).toEqual({ groupPolicy: "opt-in" }); - expect(ch?.modelByChannel).toEqual({ timbot: "gpt-3.5" }); - expect(actions.channelConfig).toBe(true); - }); - - it("does not remove shared keys even when passed as channelIds", () => { - const config: OpenClawConfig = { - plugins: { - entries: { - "bad-plugin": { enabled: true }, - }, - installs: { - "bad-plugin": { source: "npm", spec: "bad-plugin@1.0.0" }, - }, - }, - channels: { - defaults: { groupPolicy: "opt-in" }, - } as unknown as OpenClawConfig["channels"], - }; - - const { config: result, actions } = removePluginFromConfig(config, "bad-plugin", { - channelIds: ["defaults"], - }); - - const ch = result.channels as Record | undefined; - expect(ch?.defaults).toEqual({ groupPolicy: "opt-in" }); - expect(actions.channelConfig).toBe(false); - }); - - it("skips channel cleanup when channelIds is empty array", () => { - const config: OpenClawConfig = { - plugins: { - entries: { - telegram: { enabled: true }, - }, - installs: { - telegram: { source: "npm", spec: "telegram@1.0.0" }, - }, - }, - channels: { - telegram: { enabled: true }, - }, - }; - - const { config: result, actions } = removePluginFromConfig(config, "telegram", { - channelIds: [], - }); - - expect((result.channels as Record)?.telegram).toEqual({ enabled: true }); - expect(actions.channelConfig).toBe(false); - }); }); describe("uninstallPlugin", () => { diff --git a/src/plugins/uninstall.ts b/src/plugins/uninstall.ts index 62054fca65f..40fe5b90a59 100644 --- a/src/plugins/uninstall.ts +++ b/src/plugins/uninstall.ts @@ -11,7 +11,6 @@ export type UninstallActions = { allowlist: boolean; loadPath: boolean; memorySlot: boolean; - channelConfig: boolean; directory: boolean; }; @@ -59,39 +58,13 @@ export function resolveUninstallDirectoryTarget(params: { return defaultPath; } -const SHARED_CHANNEL_CONFIG_KEYS = new Set(["defaults", "modelByChannel"]); - -/** - * Resolve the channel config keys owned by a plugin during uninstall. - * - `channelIds === undefined`: fall back to the plugin id for backward compatibility. - * - `channelIds === []`: explicit "owns no channels" signal; remove nothing. - */ -export function resolveUninstallChannelConfigKeys( - pluginId: string, - opts?: { channelIds?: string[] }, -): string[] { - const rawKeys = opts?.channelIds ?? [pluginId]; - const seen = new Set(); - const keys: string[] = []; - for (const key of rawKeys) { - if (SHARED_CHANNEL_CONFIG_KEYS.has(key) || seen.has(key)) { - continue; - } - seen.add(key); - keys.push(key); - } - return keys; -} - /** * Remove plugin references from config (pure config mutation). - * Returns a new config with the plugin removed from entries, installs, allow, load.paths, slots, - * and owned channel config. + * Returns a new config with the plugin removed from entries, installs, allow, load.paths, and slots. */ export function removePluginFromConfig( cfg: OpenClawConfig, pluginId: string, - opts?: { channelIds?: string[] }, ): { config: OpenClawConfig; actions: Omit } { const actions: Omit = { entry: false, @@ -99,7 +72,6 @@ export function removePluginFromConfig( allowlist: false, loadPath: false, memorySlot: false, - channelConfig: false, }; const pluginsConfig = cfg.plugins ?? {}; @@ -156,24 +128,6 @@ export function removePluginFromConfig( slots = undefined; } - // Remove channel config owned by this installed plugin. - // Built-in channels have no install record, so keep their config untouched. - const hasInstallRecord = Object.hasOwn(cfg.plugins?.installs ?? {}, pluginId); - let channels = cfg.channels as Record | undefined; - if (hasInstallRecord && channels) { - for (const key of resolveUninstallChannelConfigKeys(pluginId, opts)) { - if (!Object.hasOwn(channels, key)) { - continue; - } - const { [key]: _removed, ...rest } = channels; - channels = Object.keys(rest).length > 0 ? rest : undefined; - actions.channelConfig = true; - if (!channels) { - break; - } - } - } - const newPlugins = { ...pluginsConfig, entries, @@ -204,7 +158,6 @@ export function removePluginFromConfig( const config: OpenClawConfig = { ...cfg, plugins: Object.keys(cleanedPlugins).length > 0 ? cleanedPlugins : undefined, - channels: channels as OpenClawConfig["channels"], }; return { config, actions }; @@ -213,7 +166,6 @@ export function removePluginFromConfig( export type UninstallPluginParams = { config: OpenClawConfig; pluginId: string; - channelIds?: string[]; deleteFiles?: boolean; extensionsDir?: string; }; @@ -225,7 +177,7 @@ export type UninstallPluginParams = { export async function uninstallPlugin( params: UninstallPluginParams, ): Promise { - const { config, pluginId, channelIds, deleteFiles = true, extensionsDir } = params; + const { config, pluginId, deleteFiles = true, extensionsDir } = params; // Validate plugin exists const hasEntry = pluginId in (config.plugins?.entries ?? {}); @@ -239,9 +191,7 @@ export async function uninstallPlugin( const isLinked = installRecord?.source === "path"; // Remove from config - const { config: newConfig, actions: configActions } = removePluginFromConfig(config, pluginId, { - channelIds, - }); + const { config: newConfig, actions: configActions } = removePluginFromConfig(config, pluginId); const actions: UninstallActions = { ...configActions,