diff --git a/src/cli/plugins-cli.uninstall.test.ts b/src/cli/plugins-cli.uninstall.test.ts index 9fb94fb449b..2aadd048134 100644 --- a/src/cli/plugins-cli.uninstall.test.ts +++ b/src/cli/plugins-cli.uninstall.test.ts @@ -271,6 +271,53 @@ describe("plugins cli uninstall", () => { }); }); + it("cleans stale policy refs even when plugin is absent from the current registry", async () => { + const baseConfig = { + plugins: { + allow: ["alpha", "beta"], + deny: ["alpha"], + }, + } as OpenClawConfig; + const nextConfig = { + plugins: { + allow: ["beta"], + }, + } as OpenClawConfig; + + loadConfig.mockReturnValue(baseConfig); + buildPluginSnapshotReport.mockReturnValue({ + plugins: [], + diagnostics: [], + }); + planPluginUninstall.mockReturnValue({ + ok: true, + config: nextConfig, + actions: { + entry: false, + install: false, + allowlist: true, + denylist: true, + loadPath: false, + memorySlot: false, + contextEngineSlot: false, + channelConfig: false, + directory: false, + }, + directoryRemoval: null, + }); + + await runPluginsCommand(["plugins", "uninstall", "alpha", "--force"]); + + expect(planPluginUninstall).toHaveBeenCalledWith( + expect.objectContaining({ + pluginId: "alpha", + deleteFiles: true, + }), + ); + expect(writeConfigFile).toHaveBeenCalledWith(nextConfig); + expect(runtimeLogs.at(-2)).toContain('Uninstalled plugin "alpha"'); + }); + it("exits when uninstall target is not managed by plugin install records", async () => { loadConfig.mockReturnValue({ plugins: { @@ -282,12 +329,16 @@ describe("plugins cli uninstall", () => { plugins: [{ id: "alpha", name: "alpha" }], diagnostics: [], }); + planPluginUninstall.mockReturnValue({ + ok: false, + error: "Plugin not found: alpha", + }); await expect(runPluginsCommand(["plugins", "uninstall", "alpha", "--force"])).rejects.toThrow( "__exit__:1", ); expect(runtimeErrors.at(-1)).toContain("is not managed by plugins config/install records"); - expect(planPluginUninstall).not.toHaveBeenCalled(); + expect(planPluginUninstall).toHaveBeenCalled(); }); }); diff --git a/src/cli/plugins-uninstall-command.ts b/src/cli/plugins-uninstall-command.ts index 8b165a03de7..d121955eba9 100644 --- a/src/cli/plugins-uninstall-command.ts +++ b/src/cli/plugins-uninstall-command.ts @@ -74,21 +74,6 @@ export async function runPluginUninstallCommand( config: cfg, plugins: report.plugins, }); - const hasEntry = pluginId in (cfg.plugins?.entries ?? {}); - const hasInstall = pluginId in (cfg.plugins?.installs ?? {}); - - if (!hasEntry && !hasInstall) { - if (plugin) { - runtime.error( - `Plugin "${pluginId}" is not managed by plugins config/install records and cannot be uninstalled.`, - ); - } else { - runtime.error(`Plugin not found: ${id}`); - } - runtime.exit(1); - return; - } - const channelIds = plugin?.status === "loaded" ? plugin.channelIds : undefined; const plan = planPluginUninstall({ config: cfg, @@ -98,10 +83,17 @@ export async function runPluginUninstallCommand( extensionsDir, }); if (!plan.ok) { - runtime.error(plan.error); + if (plugin) { + runtime.error( + `Plugin "${pluginId}" is not managed by plugins config/install records and cannot be uninstalled.`, + ); + } else { + runtime.error(plan.error); + } runtime.exit(1); return; } + const hasInstall = pluginId in (cfg.plugins?.installs ?? {}); const preview: string[] = []; if (plan.actions.entry) { diff --git a/src/plugins/uninstall.test.ts b/src/plugins/uninstall.test.ts index a5d408d93e1..237db47e319 100644 --- a/src/plugins/uninstall.test.ts +++ b/src/plugins/uninstall.test.ts @@ -704,6 +704,37 @@ describe("uninstallPlugin", () => { } }); + it("cleans stale policy references even when plugin code and install records are gone", async () => { + const result = await uninstallPlugin({ + config: createPluginConfig({ + allow: ["missing-plugin", "other-plugin"], + deny: ["missing-plugin"], + slots: { + memory: "missing-plugin", + }, + }), + pluginId: "missing-plugin", + deleteFiles: true, + }); + + const successfulResult = expectSuccessfulUninstall(result); + expect(successfulResult.actions).toEqual({ + entry: false, + install: false, + allowlist: true, + denylist: true, + loadPath: false, + memorySlot: true, + contextEngineSlot: false, + channelConfig: false, + directory: false, + }); + expect(successfulResult.config.plugins?.allow).toEqual(["other-plugin"]); + expect(successfulResult.config.plugins?.deny).toBeUndefined(); + expect(successfulResult.config.plugins?.slots?.memory).toBe("memory-core"); + expect(runCommandWithTimeoutMock).not.toHaveBeenCalled(); + }); + it("removes config entries", async () => { const config = createPluginConfig({ entries: createSinglePluginEntries(), @@ -829,6 +860,24 @@ describe("uninstallPlugin", () => { await expect(fs.access(pluginDir)).rejects.toThrow(); }); + it("skips npm cleanup when the managed package directory is already absent", async () => { + const stateDir = path.join(tempDir, "state"); + const npmRoot = path.join(stateDir, "npm"); + const pluginDir = path.join(npmRoot, "node_modules", "missing-plugin"); + + const applied = await applyPluginUninstallDirectoryRemoval({ + target: pluginDir, + cleanup: { + kind: "npm", + npmRoot, + packageName: "missing-plugin", + }, + }); + + expect(applied).toEqual({ directoryRemoved: false, warnings: [] }); + expect(runCommandWithTimeoutMock).not.toHaveBeenCalled(); + }); + it("warns and still removes npm package dirs when npm prune cleanup fails", async () => { runCommandWithTimeoutMock.mockResolvedValueOnce({ code: 1, diff --git a/src/plugins/uninstall.ts b/src/plugins/uninstall.ts index 860cc9a8e53..a01b0a9430e 100644 --- a/src/plugins/uninstall.ts +++ b/src/plugins/uninstall.ts @@ -77,6 +77,10 @@ export function formatUninstallActionLabels(actions: UninstallActions): string[] ); } +function hasUninstallAction(actions: Omit): boolean { + return Object.values(actions).some(Boolean); +} + export function formatUninstallSlotResetPreview(slotKey: "memory" | "contextEngine"): string { const actionKey = slotKey === "memory" ? "memorySlot" : "contextEngineSlot"; return `${UNINSTALL_ACTION_LABELS[actionKey]} (will reset to "${defaultSlotIdForKey(slotKey)}")`; @@ -489,14 +493,8 @@ export type UninstallPluginParams = { export function planPluginUninstall(params: UninstallPluginParams): PluginUninstallPlanResult { const { config, pluginId, channelIds, deleteFiles = true, extensionsDir } = params; - // Validate plugin exists const hasEntry = pluginId in (config.plugins?.entries ?? {}); const hasInstall = pluginId in (config.plugins?.installs ?? {}); - - if (!hasEntry && !hasInstall) { - return { ok: false, error: `Plugin not found: ${pluginId}` }; - } - const installRecord = config.plugins?.installs?.[pluginId]; const isLinked = isLinkedPathInstallRecord(installRecord); @@ -505,6 +503,10 @@ export function planPluginUninstall(params: UninstallPluginParams): PluginUninst channelIds, }); + if (!hasEntry && !hasInstall && !hasUninstallAction(configActions)) { + return { ok: false, error: `Plugin not found: ${pluginId}` }; + } + const actions: UninstallActions = { ...configActions, directory: false, @@ -563,6 +565,10 @@ export async function applyPluginUninstallDirectoryRemoval( .then(() => true) .catch(() => false)) ?? false; const warnings: string[] = []; + if (!existed) { + return { directoryRemoved: false, warnings }; + } + if (removal.cleanup?.kind === "npm") { const uninstall = await runCommandWithTimeout( [