diff --git a/CHANGELOG.md b/CHANGELOG.md index 1851db353ad..04c857a1718 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,7 @@ Docs: https://docs.openclaw.ai metadata when callers request the model without the provider prefix, so custom image models keep their `input: ["text", "image"]` capability. Fixes #33185. Thanks @Kobe9312 and @vincentkoc. +- Plugins/install: restore the previous plugin index records if a concurrent config write conflict interrupts install, update, or uninstall metadata commits. Thanks @shakkernerd. - Sessions: keep embedded runtime context out of the visible user prompt by sending it as a hidden next-turn custom message, and teach doctor to repair affected 2026.4.24 transcripts with duplicated prompt-rewrite branches. diff --git a/src/cli/plugins-cli.ts b/src/cli/plugins-cli.ts index 64c1140ded8..c390e4f9c4e 100644 --- a/src/cli/plugins-cli.ts +++ b/src/cli/plugins-cli.ts @@ -8,10 +8,8 @@ import type { PluginInstallRecord } from "../config/types.plugins.js"; import { enablePluginInConfig } from "../plugins/enable.js"; import { loadInstalledPluginIndexInstallRecords, - PLUGIN_INSTALLS_CONFIG_PATH, removePluginInstallRecordFromRecords, withoutPluginInstallRecords, - writePersistedInstalledPluginIndexInstallRecords, withPluginInstallRecords, } from "../plugins/installed-plugin-index-records.js"; import { listMarketplacePlugins } from "../plugins/marketplace.js"; @@ -44,6 +42,7 @@ import { } from "./plugins-command-helpers.js"; import { setPluginEnabledInConfig } from "./plugins-config.js"; import { runPluginInstallCommand } from "./plugins-install-command.js"; +import { commitPluginInstallRecordsWithConfig } from "./plugins-install-record-commit.js"; import { formatPluginLine } from "./plugins-list-format.js"; import { refreshPluginRegistryAfterConfigMutation } from "./plugins-registry-refresh.js"; import { resolvePluginUninstallId } from "./plugins-uninstall-selection.js"; @@ -691,17 +690,18 @@ export function registerPluginsCli(program: Command) { defaultRuntime.log(theme.warn(warning)); } - await writePersistedInstalledPluginIndexInstallRecords( - removePluginInstallRecordFromRecords(installRecords, pluginId), - ); - await replaceConfigFile({ - nextConfig: withoutPluginInstallRecords(result.config), + const nextInstallRecords = removePluginInstallRecordFromRecords(installRecords, pluginId); + const nextConfig = withoutPluginInstallRecords(result.config); + await commitPluginInstallRecordsWithConfig({ + previousInstallRecords: installRecords, + nextInstallRecords, + nextConfig, ...(snapshot.hash !== undefined ? { baseHash: snapshot.hash } : {}), - writeOptions: { unsetPaths: [Array.from(PLUGIN_INSTALLS_CONFIG_PATH)] }, }); await refreshPluginRegistryAfterConfigMutation({ - config: withoutPluginInstallRecords(result.config), + config: nextConfig, reason: "source-changed", + installRecords: nextInstallRecords, logger: { warn: (message) => defaultRuntime.log(theme.warn(message)), }, diff --git a/src/cli/plugins-cli.uninstall.test.ts b/src/cli/plugins-cli.uninstall.test.ts index c4c17ab14cd..d98f10ad34e 100644 --- a/src/cli/plugins-cli.uninstall.test.ts +++ b/src/cli/plugins-cli.uninstall.test.ts @@ -6,6 +6,7 @@ import { loadConfig, promptYesNo, refreshPluginRegistry, + replaceConfigFile, resetPluginsCliTestState, runPluginsCommand, runtimeErrors, @@ -122,6 +123,62 @@ describe("plugins cli uninstall", () => { }); }); + it("restores install records when the config write rejects during uninstall", async () => { + const installRecords = { + alpha: { + source: "path", + sourcePath: ALPHA_INSTALL_PATH, + installPath: ALPHA_INSTALL_PATH, + }, + } as const; + const baseConfig = { + plugins: { + entries: { + alpha: { enabled: true }, + }, + installs: installRecords, + }, + } as OpenClawConfig; + const nextConfig = { + plugins: { + entries: {}, + installs: {}, + }, + } as OpenClawConfig; + + loadConfig.mockReturnValue(baseConfig); + setInstalledPluginIndexInstallRecords(installRecords); + buildPluginDiagnosticsReport.mockReturnValue({ + plugins: [{ id: "alpha", name: "alpha" }], + diagnostics: [], + }); + uninstallPlugin.mockResolvedValue({ + ok: true, + config: nextConfig, + warnings: [], + actions: { + entry: true, + install: true, + allowlist: false, + loadPath: false, + memorySlot: false, + directory: false, + }, + }); + replaceConfigFile.mockRejectedValueOnce(new Error("config changed")); + + await expect( + runPluginsCommand(["plugins", "uninstall", "alpha", "--force", "--keep-files"]), + ).rejects.toThrow("config changed"); + + expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenNthCalledWith(1, {}); + expect(writePersistedInstalledPluginIndexInstallRecords).toHaveBeenNthCalledWith( + 2, + installRecords, + ); + expect(refreshPluginRegistry).not.toHaveBeenCalled(); + }); + it("exits when uninstall target is not managed by plugin install records", async () => { loadConfig.mockReturnValue({ plugins: { diff --git a/src/cli/plugins-install-persist.ts b/src/cli/plugins-install-persist.ts index fd96b240076..7a1d6973f4a 100644 --- a/src/cli/plugins-install-persist.ts +++ b/src/cli/plugins-install-persist.ts @@ -4,10 +4,8 @@ import { type HookInstallUpdate, recordHookInstall } from "../hooks/installs.js" import { enablePluginInConfig } from "../plugins/enable.js"; import { loadInstalledPluginIndexInstallRecords, - PLUGIN_INSTALLS_CONFIG_PATH, recordPluginInstallInRecords, withoutPluginInstallRecords, - writePersistedInstalledPluginIndexInstallRecords, } from "../plugins/installed-plugin-index-records.js"; import type { PluginInstallUpdate } from "../plugins/installs.js"; import { defaultRuntime } from "../runtime.js"; @@ -18,6 +16,7 @@ import { logHookPackRestartHint, logSlotWarnings, } from "./plugins-command-helpers.js"; +import { commitPluginInstallRecordsWithConfig } from "./plugins-install-record-commit.js"; import { refreshPluginRegistryAfterConfigMutation } from "./plugins-registry-refresh.js"; function addInstalledPluginToAllowlist(cfg: OpenClawConfig, pluginId: string): OpenClawConfig { @@ -53,11 +52,11 @@ export async function persistPluginInstall(params: { }); const slotResult = applySlotSelectionForPlugin(next, params.pluginId); next = withoutPluginInstallRecords(slotResult.config); - await writePersistedInstalledPluginIndexInstallRecords(nextInstallRecords); - await replaceConfigFile({ + await commitPluginInstallRecordsWithConfig({ + previousInstallRecords: installRecords, + nextInstallRecords, nextConfig: next, ...(params.baseHash !== undefined ? { baseHash: params.baseHash } : {}), - writeOptions: { unsetPaths: [Array.from(PLUGIN_INSTALLS_CONFIG_PATH)] }, }); await refreshPluginRegistryAfterConfigMutation({ config: next, diff --git a/src/cli/plugins-install-record-commit.ts b/src/cli/plugins-install-record-commit.ts new file mode 100644 index 00000000000..cf143675062 --- /dev/null +++ b/src/cli/plugins-install-record-commit.ts @@ -0,0 +1,33 @@ +import { replaceConfigFile } from "../config/config.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import type { PluginInstallRecord } from "../config/types.plugins.js"; +import { + PLUGIN_INSTALLS_CONFIG_PATH, + writePersistedInstalledPluginIndexInstallRecords, +} from "../plugins/installed-plugin-index-records.js"; + +export async function commitPluginInstallRecordsWithConfig(params: { + previousInstallRecords: Record; + nextInstallRecords: Record; + nextConfig: OpenClawConfig; + baseHash?: string; +}): Promise { + await writePersistedInstalledPluginIndexInstallRecords(params.nextInstallRecords); + try { + await replaceConfigFile({ + nextConfig: params.nextConfig, + ...(params.baseHash !== undefined ? { baseHash: params.baseHash } : {}), + writeOptions: { unsetPaths: [Array.from(PLUGIN_INSTALLS_CONFIG_PATH)] }, + }); + } catch (error) { + try { + await writePersistedInstalledPluginIndexInstallRecords(params.previousInstallRecords); + } catch (rollbackError) { + throw new Error( + "Failed to commit plugin install records and could not restore the previous plugin index", + { cause: rollbackError }, + ); + } + throw error; + } +} diff --git a/src/cli/plugins-update-command.ts b/src/cli/plugins-update-command.ts index 0e648d9e09d..031f2bd7d7d 100644 --- a/src/cli/plugins-update-command.ts +++ b/src/cli/plugins-update-command.ts @@ -2,14 +2,13 @@ import { loadConfig, readConfigFileSnapshot, replaceConfigFile } from "../config import { updateNpmInstalledHookPacks } from "../hooks/update.js"; import { loadInstalledPluginIndexInstallRecords, - PLUGIN_INSTALLS_CONFIG_PATH, withoutPluginInstallRecords, - writePersistedInstalledPluginIndexInstallRecords, withPluginInstallRecords, } from "../plugins/installed-plugin-index-records.js"; import { updateNpmInstalledPlugins } from "../plugins/update.js"; import { defaultRuntime } from "../runtime.js"; import { theme } from "../terminal/theme.js"; +import { commitPluginInstallRecordsWithConfig } from "./plugins-install-record-commit.js"; import { refreshPluginRegistryAfterConfigMutation } from "./plugins-registry-refresh.js"; import { resolveHookPackUpdateSelection, @@ -121,19 +120,22 @@ export async function runPluginUpdateCommand(params: { const nextPluginInstallRecords = pluginResult.config.plugins?.installs ?? {}; const shouldPersistPluginInstallIndex = pluginResult.changed || Object.keys(pluginInstallRecords).length > 0; - if (shouldPersistPluginInstallIndex) { - await writePersistedInstalledPluginIndexInstallRecords(nextPluginInstallRecords); - } const nextConfig = shouldPersistPluginInstallIndex ? withoutPluginInstallRecords(hookResult.config) : hookResult.config; - await replaceConfigFile({ - nextConfig, - baseHash: (await sourceSnapshotPromise)?.hash, - ...(shouldPersistPluginInstallIndex - ? { writeOptions: { unsetPaths: [Array.from(PLUGIN_INSTALLS_CONFIG_PATH)] } } - : {}), - }); + if (shouldPersistPluginInstallIndex) { + await commitPluginInstallRecordsWithConfig({ + previousInstallRecords: pluginInstallRecords, + nextInstallRecords: nextPluginInstallRecords, + nextConfig, + baseHash: (await sourceSnapshotPromise)?.hash, + }); + } else { + await replaceConfigFile({ + nextConfig, + baseHash: (await sourceSnapshotPromise)?.hash, + }); + } if (pluginResult.changed) { await refreshPluginRegistryAfterConfigMutation({ config: nextConfig,