diff --git a/CHANGELOG.md b/CHANGELOG.md index 82dd2cc87f0..2d7ddf223b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Plugins/install: remove the previous managed plugin directory when a reinstall switches sources, so stale ClawHub and npm copies no longer keep duplicate plugin ids in discovery after the new install wins. Thanks @vincentkoc. - Plugins/install: let official plugin reinstall recovery repair source-only installed runtime shadows, so `openclaw plugins install npm:@openclaw/discord --force` can replace the bad package instead of stopping at stale config validation. Thanks @vincentkoc. - Plugins/commands: allow the official ClawHub Codex plugin package to keep reserved `/codex` command ownership, matching the existing npm-managed Codex package behavior. Thanks @vincentkoc. - Plugins/commands: scope QQBot framework slash commands to the QQBot channel so `/bot-*` command handlers and native specs do not leak onto unrelated chat surfaces. Thanks @vincentkoc. diff --git a/src/cli/plugins-install-persist.test.ts b/src/cli/plugins-install-persist.test.ts index b981171a6b3..6c7ce2d54bf 100644 --- a/src/cli/plugins-install-persist.test.ts +++ b/src/cli/plugins-install-persist.test.ts @@ -7,12 +7,15 @@ import { clearPluginRegistryLoadCache, enablePluginInConfig, loadPluginManifestRegistry, + planPluginUninstall, replaceConfigFile, refreshPluginRegistry, resetPluginsCliTestState, runtimeLogs, + setInstalledPluginIndexInstallRecords, writeConfigFile, writePersistedInstalledPluginIndexInstallRecords, + applyPluginUninstallDirectoryRemoval, } from "./plugins-cli-test-helpers.js"; describe("persistPluginInstall", () => { @@ -125,6 +128,132 @@ describe("persistPluginInstall", () => { ).toBe(true); }); + it("removes a replaced managed install directory before refreshing the registry", async () => { + const { persistPluginInstall } = await import("./plugins-install-persist.js"); + const baseConfig = { + plugins: { + entries: {}, + }, + } as OpenClawConfig; + const enabledConfig = { + plugins: { + entries: { + codex: { enabled: true }, + }, + }, + } as OpenClawConfig; + enablePluginInConfig.mockReturnValue({ config: enabledConfig }); + setInstalledPluginIndexInstallRecords({ + codex: { + source: "clawhub", + spec: "clawhub:@openclaw/codex", + installPath: "/tmp/openclaw/extensions/codex", + }, + }); + planPluginUninstall.mockReturnValueOnce({ + ok: true, + config: {} as OpenClawConfig, + pluginId: "codex", + actions: { + entry: false, + install: true, + allowlist: false, + denylist: false, + loadPath: false, + memorySlot: false, + contextEngineSlot: false, + channelConfig: false, + directory: false, + }, + directoryRemoval: { + target: "/tmp/openclaw/extensions/codex", + }, + }); + applyPluginUninstallDirectoryRemoval.mockResolvedValueOnce({ + directoryRemoved: true, + warnings: [], + }); + + await persistPluginInstall({ + snapshot: { + config: baseConfig, + baseHash: "config-1", + }, + pluginId: "codex", + install: { + source: "npm", + spec: "@openclaw/codex", + installPath: "/tmp/openclaw/npm/node_modules/@openclaw/codex", + }, + }); + + expect(planPluginUninstall).toHaveBeenCalledWith({ + config: { + plugins: { + installs: { + codex: { + source: "clawhub", + spec: "clawhub:@openclaw/codex", + installPath: "/tmp/openclaw/extensions/codex", + }, + }, + }, + }, + pluginId: "codex", + deleteFiles: true, + }); + expect(applyPluginUninstallDirectoryRemoval).toHaveBeenCalledWith({ + target: "/tmp/openclaw/extensions/codex", + }); + const cleanupOrder = + applyPluginUninstallDirectoryRemoval.mock.invocationCallOrder[0] ?? Number.MAX_SAFE_INTEGER; + const refreshOrder = refreshPluginRegistry.mock.invocationCallOrder[0] ?? 0; + expect(cleanupOrder).toBeLessThan(refreshOrder); + expect(runtimeLogs.join("\n")).toContain( + "Removed previous plugin install directory: /tmp/openclaw/extensions/codex", + ); + }); + + it("preserves replaced install directories when the new install path overlaps", async () => { + const { persistPluginInstall } = await import("./plugins-install-persist.js"); + const baseConfig = { + plugins: { + entries: {}, + }, + } as OpenClawConfig; + const enabledConfig = { + plugins: { + entries: { + codex: { enabled: true }, + }, + }, + } as OpenClawConfig; + enablePluginInConfig.mockReturnValue({ config: enabledConfig }); + setInstalledPluginIndexInstallRecords({ + codex: { + source: "npm", + spec: "@openclaw/codex", + installPath: "/tmp/openclaw/npm/node_modules/@openclaw/codex", + }, + }); + + await persistPluginInstall({ + snapshot: { + config: baseConfig, + baseHash: "config-1", + }, + pluginId: "codex", + install: { + source: "npm", + spec: "@openclaw/codex@latest", + installPath: "/tmp/openclaw/npm/node_modules/@openclaw/codex", + }, + }); + + expect(planPluginUninstall).not.toHaveBeenCalled(); + expect(applyPluginUninstallDirectoryRemoval).not.toHaveBeenCalled(); + }); + it("warns when an installed npm plugin remains shadowed by a config-selected source", async () => { const { persistPluginInstall } = await import("./plugins-install-persist.js"); const baseConfig = { diff --git a/src/cli/plugins-install-persist.ts b/src/cli/plugins-install-persist.ts index 4c787c1b5c4..efe7885e872 100644 --- a/src/cli/plugins-install-persist.ts +++ b/src/cli/plugins-install-persist.ts @@ -1,5 +1,6 @@ import { replaceConfigFile } from "../config/config.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; +import type { PluginInstallRecord } from "../config/types.plugins.js"; import { type HookInstallUpdate, recordHookInstall } from "../hooks/installs.js"; import { isPathInside } from "../infra/path-guards.js"; import { enablePluginInConfig } from "../plugins/enable.js"; @@ -11,6 +12,11 @@ import { import type { PluginInstallUpdate } from "../plugins/installs.js"; import { tracePluginLifecyclePhaseAsync } from "../plugins/plugin-lifecycle-trace.js"; import { buildPluginSnapshotReport } from "../plugins/status.js"; +import { + applyPluginUninstallDirectoryRemoval, + planPluginUninstall, + type PluginUninstallDirectoryRemoval, +} from "../plugins/uninstall.js"; import { defaultRuntime, type RuntimeEnv } from "../runtime.js"; import { theme } from "../terminal/theme.js"; import { resolveUserPath, shortenHomePath } from "../utils.js"; @@ -110,6 +116,69 @@ function logShadowedNpmInstallWarning(params: { ); } +function resolveComparableInstallPath( + install: Pick, +) { + return install.installPath ?? install.sourcePath; +} + +function shouldPreserveReplacedInstallPath(params: { + removalTarget: string; + nextInstallPath: string; +}) { + const removalTarget = resolveUserPath(params.removalTarget); + const nextInstallPath = resolveUserPath(params.nextInstallPath); + return ( + isPathInside(removalTarget, nextInstallPath) || isPathInside(nextInstallPath, removalTarget) + ); +} + +function resolveReplacedManagedInstallRemoval(params: { + pluginId: string; + previousInstall?: PluginInstallRecord; + nextInstall: Omit; +}): PluginUninstallDirectoryRemoval | null { + if (!params.previousInstall) { + return null; + } + const previousInstallPath = resolveComparableInstallPath(params.previousInstall); + const nextInstallPath = resolveComparableInstallPath(params.nextInstall); + if (!previousInstallPath || !nextInstallPath) { + return null; + } + if ( + shouldPreserveReplacedInstallPath({ + removalTarget: previousInstallPath, + nextInstallPath, + }) + ) { + return null; + } + const plan = planPluginUninstall({ + config: { + plugins: { + installs: { + [params.pluginId]: params.previousInstall, + }, + }, + } as OpenClawConfig, + pluginId: params.pluginId, + deleteFiles: true, + }); + if (!plan.ok || !plan.directoryRemoval) { + return null; + } + if ( + shouldPreserveReplacedInstallPath({ + removalTarget: plan.directoryRemoval.target, + nextInstallPath, + }) + ) { + return null; + } + return plan.directoryRemoval; +} + export async function persistPluginInstall(params: { snapshot: ConfigSnapshotForInstallPersist; pluginId: string; @@ -138,6 +207,11 @@ export async function persistPluginInstall(params: { () => loadInstalledPluginIndexInstallRecords(), { command: "install" }, ); + const replacedInstallRemoval = resolveReplacedManagedInstallRemoval({ + pluginId: params.pluginId, + previousInstall: installRecords[params.pluginId], + nextInstall: params.install, + }); const nextInstallRecords = recordPluginInstallInRecords(installRecords, { pluginId: params.pluginId, ...params.install, @@ -165,6 +239,23 @@ export async function persistPluginInstall(params: { }), { command: "install" }, ); + if (replacedInstallRemoval) { + const removalResult = await tracePluginLifecyclePhaseAsync( + "replaced install cleanup", + () => applyPluginUninstallDirectoryRemoval(replacedInstallRemoval), + { command: "install", pluginId: params.pluginId }, + ); + for (const warning of removalResult.warnings) { + runtime.log(theme.warn(warning)); + } + if (removalResult.directoryRemoved) { + runtime.log( + theme.muted( + `Removed previous plugin install directory: ${shortenHomePath(replacedInstallRemoval.target)}`, + ), + ); + } + } await refreshPluginRegistryAfterConfigMutation({ config: next, reason: "source-changed",