mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 10:10:45 +00:00
fix: roll back plugin index on config write conflicts
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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)),
|
||||
},
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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,
|
||||
|
||||
33
src/cli/plugins-install-record-commit.ts
Normal file
33
src/cli/plugins-install-record-commit.ts
Normal file
@@ -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<string, PluginInstallRecord>;
|
||||
nextInstallRecords: Record<string, PluginInstallRecord>;
|
||||
nextConfig: OpenClawConfig;
|
||||
baseHash?: string;
|
||||
}): Promise<void> {
|
||||
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;
|
||||
}
|
||||
}
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user