mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 08:10:44 +00:00
fix: hot reload plugin management changes (#75976)
Summary: - The PR changes Gateway reload planning, CLI plugin install-index writes, plugin runtime/cache cleanup, docs, changelog, and tests so plugin enable/disable hot reloads while install/update/uninstall stay restart-backed. - Reproducibility: yes. The earlier blocker has a source-level reproduction: run an external plugin install/up ... watches config and only the managed plugin index changes; the PR now tests that path and queues a restart. ClawSweeper fixups: - Included follow-up commit: fix: hot reload plugin management changes - Included follow-up commit: fix(clawsweeper): address review for automerge-openclaw-openclaw-7597… - Ran the ClawSweeper repair loop before final review. Validation: - ClawSweeper review passed for head860594f722. - Required merge gates passed before the squash merge. Prepared head SHA:860594f722Review: https://github.com/openclaw/openclaw/pull/75976#issuecomment-4363168379 Co-authored-by: Peter Steinberger <steipete@gmail.com> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
c9fa7b61f1
commit
d678bcfcc7
@@ -66,6 +66,7 @@ export const buildPluginDiagnosticsReport: UnknownMock = vi.fn();
|
||||
const buildPluginCompatibilityNotices: UnknownMock = vi.fn();
|
||||
export const inspectPluginRegistry: AsyncUnknownMock = vi.fn();
|
||||
export const refreshPluginRegistry: AsyncUnknownMock = vi.fn();
|
||||
export const clearPluginRegistryLoadCache: UnknownMock = vi.fn();
|
||||
export const applyExclusiveSlotSelection: UnknownMock = vi.fn();
|
||||
export const planPluginUninstall: UnknownMock = vi.fn();
|
||||
export const applyPluginUninstallDirectoryRemoval: AsyncUnknownMock = vi.fn();
|
||||
@@ -353,6 +354,13 @@ vi.mock("../plugins/plugin-registry.js", () => ({
|
||||
)) as (typeof import("../plugins/plugin-registry.js"))["refreshPluginRegistry"],
|
||||
}));
|
||||
|
||||
vi.mock("../plugins/loader.js", () => ({
|
||||
clearPluginRegistryLoadCache: ((...args: unknown[]) =>
|
||||
invokeMock<unknown[], unknown>(clearPluginRegistryLoadCache, ...args)) as (
|
||||
...args: unknown[]
|
||||
) => unknown,
|
||||
}));
|
||||
|
||||
vi.mock("../plugins/slots.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../plugins/slots.js")>();
|
||||
return {
|
||||
@@ -599,6 +607,7 @@ export function resetPluginsCliTestState() {
|
||||
buildPluginCompatibilityNotices.mockReset();
|
||||
inspectPluginRegistry.mockReset();
|
||||
refreshPluginRegistry.mockReset();
|
||||
clearPluginRegistryLoadCache.mockReset();
|
||||
applyExclusiveSlotSelection.mockReset();
|
||||
planPluginUninstall.mockReset();
|
||||
applyPluginUninstallDirectoryRemoval.mockReset();
|
||||
|
||||
@@ -3,10 +3,13 @@ import type { OpenClawConfig } from "../config/config.js";
|
||||
import {
|
||||
applyExclusiveSlotSelection,
|
||||
buildPluginDiagnosticsReport,
|
||||
clearPluginRegistryLoadCache,
|
||||
enablePluginInConfig,
|
||||
loadPluginManifestRegistry,
|
||||
replaceConfigFile,
|
||||
refreshPluginRegistry,
|
||||
resetPluginsCliTestState,
|
||||
runtimeLogs,
|
||||
writeConfigFile,
|
||||
writePersistedInstalledPluginIndexInstallRecords,
|
||||
} from "./plugins-cli-test-helpers.js";
|
||||
@@ -60,6 +63,14 @@ describe("persistPluginInstall", () => {
|
||||
}),
|
||||
});
|
||||
expect(writeConfigFile).toHaveBeenCalledWith(enabledConfig);
|
||||
expect(replaceConfigFile).toHaveBeenCalledWith({
|
||||
nextConfig: enabledConfig,
|
||||
baseHash: "config-1",
|
||||
writeOptions: {
|
||||
afterWrite: { mode: "restart", reason: "plugin source changed" },
|
||||
unsetPaths: [["plugins", "installs"]],
|
||||
},
|
||||
});
|
||||
expect(refreshPluginRegistry).toHaveBeenCalledWith({
|
||||
config: enabledConfig,
|
||||
installRecords: {
|
||||
@@ -71,6 +82,82 @@ describe("persistPluginInstall", () => {
|
||||
},
|
||||
reason: "source-changed",
|
||||
});
|
||||
expect(clearPluginRegistryLoadCache).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("persists installs even when runtime cache invalidation fails", async () => {
|
||||
const { persistPluginInstall } = await import("./plugins-install-persist.js");
|
||||
const baseConfig = {
|
||||
plugins: {
|
||||
entries: {},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
const enabledConfig = {
|
||||
plugins: {
|
||||
entries: {
|
||||
alpha: { enabled: true },
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
enablePluginInConfig.mockReturnValue({ config: enabledConfig });
|
||||
clearPluginRegistryLoadCache.mockImplementation(() => {
|
||||
throw new Error("cache unavailable");
|
||||
});
|
||||
|
||||
const next = await persistPluginInstall({
|
||||
snapshot: {
|
||||
config: baseConfig,
|
||||
baseHash: "config-1",
|
||||
},
|
||||
pluginId: "alpha",
|
||||
install: {
|
||||
source: "npm",
|
||||
spec: "alpha@1.0.0",
|
||||
installPath: "/tmp/alpha",
|
||||
},
|
||||
});
|
||||
|
||||
expect(next).toEqual(enabledConfig);
|
||||
expect(refreshPluginRegistry).toHaveBeenCalled();
|
||||
expect(
|
||||
runtimeLogs.some((line) => line.includes("Plugin runtime cache invalidation failed")),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("invalidates runtime cache even when registry refresh fails", async () => {
|
||||
const { persistPluginInstall } = await import("./plugins-install-persist.js");
|
||||
const baseConfig = {
|
||||
plugins: {
|
||||
entries: {},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
const enabledConfig = {
|
||||
plugins: {
|
||||
entries: {
|
||||
alpha: { enabled: true },
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
enablePluginInConfig.mockReturnValue({ config: enabledConfig });
|
||||
refreshPluginRegistry.mockRejectedValueOnce(new Error("registry unavailable"));
|
||||
|
||||
const next = await persistPluginInstall({
|
||||
snapshot: {
|
||||
config: baseConfig,
|
||||
baseHash: "config-1",
|
||||
},
|
||||
pluginId: "alpha",
|
||||
install: {
|
||||
source: "npm",
|
||||
spec: "alpha@1.0.0",
|
||||
installPath: "/tmp/alpha",
|
||||
},
|
||||
});
|
||||
|
||||
expect(next).toEqual(enabledConfig);
|
||||
expect(refreshPluginRegistry).toHaveBeenCalled();
|
||||
expect(clearPluginRegistryLoadCache).toHaveBeenCalledTimes(1);
|
||||
expect(runtimeLogs.some((line) => line.includes("Plugin registry refresh failed"))).toBe(true);
|
||||
});
|
||||
|
||||
it("removes stale denylist entries before enabling installed plugins", async () => {
|
||||
|
||||
@@ -107,6 +107,9 @@ export async function persistPluginInstall(params: {
|
||||
nextInstallRecords,
|
||||
nextConfig: next,
|
||||
baseHash: params.snapshot.baseHash,
|
||||
writeOptions: {
|
||||
afterWrite: { mode: "restart", reason: "plugin source changed" },
|
||||
},
|
||||
}),
|
||||
{ command: "install" },
|
||||
);
|
||||
|
||||
@@ -78,6 +78,7 @@ describe("commitConfigWithPendingPluginInstalls", () => {
|
||||
},
|
||||
baseHash: "config-1",
|
||||
writeOptions: {
|
||||
afterWrite: { mode: "restart", reason: "plugin source changed" },
|
||||
unsetPaths: [["plugins", "installs"]],
|
||||
},
|
||||
});
|
||||
@@ -97,6 +98,33 @@ describe("commitConfigWithPendingPluginInstalls", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("does not add restart intent when pending records match the plugin index", async () => {
|
||||
const existingRecords: Record<string, PluginInstallRecord> = {
|
||||
demo: {
|
||||
source: "npm",
|
||||
spec: "demo@1.0.0",
|
||||
},
|
||||
};
|
||||
mocks.loadInstalledPluginIndexInstallRecords.mockResolvedValue(existingRecords);
|
||||
|
||||
await commitConfigWithPendingPluginInstalls({
|
||||
nextConfig: {
|
||||
plugins: {
|
||||
installs: existingRecords,
|
||||
},
|
||||
},
|
||||
baseHash: "config-1",
|
||||
});
|
||||
|
||||
expect(mocks.replaceConfigFile).toHaveBeenCalledWith({
|
||||
nextConfig: {},
|
||||
baseHash: "config-1",
|
||||
writeOptions: {
|
||||
unsetPaths: [["plugins", "installs"]],
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("rolls back plugin index writes when the config write fails", async () => {
|
||||
const existingRecords: Record<string, PluginInstallRecord> = {
|
||||
existing: {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { isDeepStrictEqual } from "node:util";
|
||||
import { replaceConfigFile } from "../config/config.js";
|
||||
import type { ConfigWriteOptions } from "../config/io.js";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
@@ -18,6 +19,7 @@ function mergeUnsetPaths(
|
||||
}
|
||||
|
||||
type ConfigCommit = (config: OpenClawConfig, writeOptions?: ConfigWriteOptions) => Promise<void>;
|
||||
const PLUGIN_SOURCE_CHANGED_RESTART_REASON = "plugin source changed";
|
||||
|
||||
async function commitPluginInstallRecordsWithWriter(params: {
|
||||
previousInstallRecords?: Record<string, PluginInstallRecord>;
|
||||
@@ -30,8 +32,15 @@ async function commitPluginInstallRecordsWithWriter(params: {
|
||||
params.previousInstallRecords ?? (await loadInstalledPluginIndexInstallRecords());
|
||||
await writePersistedInstalledPluginIndexInstallRecords(params.nextInstallRecords);
|
||||
try {
|
||||
const installRecordsChanged = !isDeepStrictEqual(
|
||||
previousInstallRecords,
|
||||
params.nextInstallRecords,
|
||||
);
|
||||
await params.commit(params.nextConfig, {
|
||||
...params.writeOptions,
|
||||
...(installRecordsChanged && params.writeOptions?.afterWrite === undefined
|
||||
? { afterWrite: { mode: "restart", reason: PLUGIN_SOURCE_CHANGED_RESTART_REASON } }
|
||||
: {}),
|
||||
unsetPaths: mergeUnsetPaths(params.writeOptions?.unsetPaths, [
|
||||
Array.from(PLUGIN_INSTALLS_CONFIG_PATH),
|
||||
]),
|
||||
|
||||
@@ -43,4 +43,16 @@ export async function refreshPluginRegistryAfterConfigMutation(params: {
|
||||
} catch (error) {
|
||||
params.logger?.warn?.(`Plugin registry refresh failed: ${formatErrorMessage(error)}`);
|
||||
}
|
||||
await invalidatePluginRuntimeDiscoveryAfterConfigMutation(params);
|
||||
}
|
||||
|
||||
async function invalidatePluginRuntimeDiscoveryAfterConfigMutation(params: {
|
||||
logger?: PluginRegistryRefreshLogger;
|
||||
}): Promise<void> {
|
||||
try {
|
||||
const { clearPluginRegistryLoadCache } = await import("../plugins/loader.js");
|
||||
clearPluginRegistryLoadCache();
|
||||
} catch (error) {
|
||||
params.logger?.warn?.(`Plugin runtime cache invalidation failed: ${formatErrorMessage(error)}`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -167,6 +167,9 @@ export async function runPluginUninstallCommand(
|
||||
nextInstallRecords,
|
||||
nextConfig,
|
||||
...(snapshot.hash !== undefined ? { baseHash: snapshot.hash } : {}),
|
||||
writeOptions: {
|
||||
afterWrite: { mode: "restart", reason: "plugin source changed" },
|
||||
},
|
||||
}),
|
||||
{ command: "uninstall" },
|
||||
);
|
||||
|
||||
@@ -111,6 +111,9 @@ export async function runPluginUpdateCommand(params: {
|
||||
nextInstallRecords: nextPluginInstallRecords,
|
||||
nextConfig,
|
||||
baseHash: (await sourceSnapshotPromise)?.hash,
|
||||
writeOptions: {
|
||||
afterWrite: { mode: "restart", reason: "plugin source changed" },
|
||||
},
|
||||
});
|
||||
} else {
|
||||
await replaceConfigFile({
|
||||
|
||||
Reference in New Issue
Block a user