mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:20:43 +00:00
fix(plugins): clean replaced managed installs
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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<PluginInstallRecord, "installPath" | "sourcePath">,
|
||||
) {
|
||||
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<PluginInstallUpdate, "pluginId">;
|
||||
}): 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",
|
||||
|
||||
Reference in New Issue
Block a user