From 0f343ad568adc43cace2eb7a4ad3c94e57aa1fe9 Mon Sep 17 00:00:00 2001 From: Shakker Date: Sun, 26 Apr 2026 02:56:30 +0100 Subject: [PATCH] fix: make plugin install config migration atomic --- CHANGELOG.md | 1 + src/config/io.ts | 95 +++++++++++++++++++++++++++--- src/config/io.write-config.test.ts | 41 +++++++++++++ 3 files changed, 129 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5ae979d7ae..720b203846e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -150,6 +150,7 @@ Docs: https://docs.openclaw.ai - Plugins/onboarding: defer channel/provider plugin install records until the owning config write commits, keeping setup failures from advancing the plugin index ahead of `openclaw.json`. Thanks @shakkernerd. - Plugins/config: route configure and agent setup writes with pending plugin install records through the plugin index commit helper so provider onboarding metadata is not stripped by plain config writes. Thanks @shakkernerd. - Plugins/channels: merge pending channel plugin install records with the existing plugin index before config writes, preserving unrelated tracked installs during channel setup, resolve, remove, and capability repair flows. Thanks @shakkernerd. +- Plugins/config: defer shipped `plugins.installs` index migration during config writes until the guarded config commit window and roll it back if the config write fails before commit. 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/config/io.ts b/src/config/io.ts index 5c7fbe66890..26e1f6aab9d 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -20,6 +20,7 @@ import { } from "../plugins/doctor-contract-registry.js"; import { loadInstalledPluginIndexInstallRecordsSync, + resolveInstalledPluginIndexRecordsStorePath, writePersistedInstalledPluginIndexInstallRecordsSync, } from "../plugins/installed-plugin-index-records.js"; import { sanitizeTerminalText } from "../terminal/safe-text.js"; @@ -118,6 +119,23 @@ export { CircularIncludeError, ConfigIncludeError } from "./includes.js"; export { MissingEnvVarError } from "./env-substitution.js"; export { resolveShellEnvExpectedKeys } from "./shell-env-expected-keys.js"; +type ShippedPluginInstallConfigWriteMigration = + | { + migrated: false; + } + | { + migrated: true; + filePath: string; + previousFile: + | { + existed: false; + } + | { + existed: true; + raw: string; + }; + }; + const CONFIG_HEALTH_STATE_FILENAME = "config-health.json"; const loggedInvalidConfigs = new Set(); @@ -1210,12 +1228,18 @@ export function createConfigIO( return applyConfigOverrides(cfgWithOwnerDisplaySecret); } - function migrateAndStripShippedPluginInstallConfigRecords(configRaw: unknown): unknown { + function migrateAndStripShippedPluginInstallConfigRecords( + configRaw: unknown, + options: { persist?: boolean } = {}, + ): unknown { const installRecords = extractShippedPluginInstallConfigRecords(configRaw); const stripped = stripShippedPluginInstallConfigRecords(configRaw); if (Object.keys(installRecords).length === 0) { return stripped; } + if (options.persist === false) { + return stripped; + } try { const stateDir = resolveStateDir(deps.env, deps.homedir); @@ -1248,24 +1272,34 @@ export function createConfigIO( function ensureShippedPluginInstallConfigRecordsMigratedForWrite( snapshot: ConfigFileSnapshot, - ): void { + ): ShippedPluginInstallConfigWriteMigration { const installRecords = { ...extractShippedPluginInstallConfigRecords(snapshot.sourceConfig), ...extractShippedPluginInstallConfigRecords(snapshot.parsed), }; if (Object.keys(installRecords).length === 0) { - return; + return { migrated: false }; } const stateDir = resolveStateDir(deps.env, deps.homedir); + const filePath = resolveInstalledPluginIndexRecordsStorePath({ + env: deps.env, + stateDir, + }); const existingRecords = loadInstalledPluginIndexInstallRecordsSync({ env: deps.env, stateDir, }); if (Object.keys(installRecords).every((pluginId) => pluginId in existingRecords)) { - return; + return { migrated: false }; } + const previousFile = deps.fs.existsSync(filePath) + ? ({ + existed: true, + raw: deps.fs.readFileSync(filePath, "utf-8"), + } as const) + : ({ existed: false } as const); try { writePersistedInstalledPluginIndexInstallRecordsSync( { @@ -1278,6 +1312,11 @@ export function createConfigIO( stateDir, }, ); + return { + migrated: true, + filePath, + previousFile, + }; } catch (err) { throw new Error( `Config write blocked: shipped plugins.installs records in ${configPath} could not be migrated into the plugin index. Fix state directory permissions or run openclaw plugins registry --refresh, then retry. ${formatErrorMessage( @@ -1288,6 +1327,28 @@ export function createConfigIO( } } + function rollbackShippedPluginInstallConfigWriteMigration( + migration: ShippedPluginInstallConfigWriteMigration, + ): void { + if (!migration.migrated) { + return; + } + if (migration.previousFile.existed) { + deps.fs.writeFileSync(migration.filePath, migration.previousFile.raw, { + encoding: "utf-8", + mode: 0o600, + }); + return; + } + try { + deps.fs.unlinkSync(migration.filePath); + } catch (err) { + if ((err as NodeJS.ErrnoException)?.code !== "ENOENT") { + throw err; + } + } + } + function loadConfig(): OpenClawConfig { try { maybeLoadDotEnvForConfig(deps.env); @@ -1423,7 +1484,9 @@ export function createConfigIO( } } - async function readConfigFileSnapshotInternal(): Promise { + async function readConfigFileSnapshotInternal( + options: { persistShippedPluginInstallMigration?: boolean } = {}, + ): Promise { maybeLoadDotEnvForConfig(deps.env); const exists = deps.fs.existsSync(configPath); if (!exists) { @@ -1533,6 +1596,7 @@ export function createConfigIO( const legacyResolution = resolveLegacyConfigForRead(resolvedConfigRaw, effectiveParsed); const effectiveConfigRaw = migrateAndStripShippedPluginInstallConfigRecords( legacyResolution.effectiveConfigRaw, + { persist: options.persistShippedPluginInstallMigration !== false }, ); fallbackSourceConfig = coerceConfig(effectiveConfigRaw); const validated = validateConfigObjectWithPlugins(effectiveConfigRaw, { @@ -1650,7 +1714,9 @@ export function createConfigIO( } async function readConfigFileSnapshotForWrite(): Promise { - const result = await readConfigFileSnapshotInternal(); + const result = await readConfigFileSnapshotInternal({ + persistShippedPluginInstallMigration: false, + }); return { snapshot: result.snapshot, writeOptions: { @@ -1719,8 +1785,13 @@ export function createConfigIO( clearConfigCache(); const unsetPaths = resolveManagedUnsetPathsForWrite(options.unsetPaths); let persistCandidate: unknown = cfg; - const snapshot = options.baseSnapshot ?? (await readConfigFileSnapshotInternal()).snapshot; - ensureShippedPluginInstallConfigRecordsMigratedForWrite(snapshot); + const snapshot = + options.baseSnapshot ?? + ( + await readConfigFileSnapshotInternal({ + persistShippedPluginInstallMigration: false, + }) + ).snapshot; let envRefMap: Map | null = null; let changedPaths: Set | null = null; if (snapshot.valid && snapshot.exists) { @@ -1938,6 +2009,9 @@ export function createConfigIO( `${path.basename(configPath)}.${process.pid}.${crypto.randomUUID()}.tmp`, ); + const pluginInstallConfigMigration = + ensureShippedPluginInstallConfigRecordsMigratedForWrite(snapshot); + let configCommitted = false; try { await deps.fs.promises.writeFile(tmp, json, { encoding: "utf-8", @@ -1961,6 +2035,7 @@ export function createConfigIO( await deps.fs.promises.unlink(tmp).catch(() => { // best-effort }); + configCommitted = true; logConfigOverwrite(); logConfigWriteAnomalies(); await appendWriteAudit( @@ -1975,6 +2050,7 @@ export function createConfigIO( }); throw err; } + configCommitted = true; logConfigOverwrite(); logConfigWriteAnomalies(); await appendWriteAudit( @@ -1984,6 +2060,9 @@ export function createConfigIO( ); return { persistedHash: nextHash, persistedConfig: stampedOutputConfig }; } catch (err) { + if (!configCommitted) { + rollbackShippedPluginInstallConfigWriteMigration(pluginInstallConfigMigration); + } await appendWriteAudit("failed", err); throw err; } diff --git a/src/config/io.write-config.test.ts b/src/config/io.write-config.test.ts index 7e4fcc02f93..36db1e1a041 100644 --- a/src/config/io.write-config.test.ts +++ b/src/config/io.write-config.test.ts @@ -75,6 +75,8 @@ describe("config io write", () => { afterEach(() => { resetConfigRuntimeState(); + mockMaintainConfigBackups.mockReset(); + mockMaintainConfigBackups.mockResolvedValue(undefined); }); afterAll(async () => { @@ -280,6 +282,45 @@ describe("config io write", () => { }); }); + it("rolls back shipped plugin install index migration when config write fails", async () => { + await withSuiteHome(async (home) => { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + const pluginDir = path.join(home, ".openclaw", "plugins", "demo"); + const original = { + plugins: { + entries: { demo: { enabled: true } }, + installs: { + demo: { + source: "npm", + spec: "demo@1.0.0", + installPath: pluginDir, + }, + }, + }, + }; + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile(configPath, `${JSON.stringify(original, null, 2)}\n`, "utf-8"); + mockMaintainConfigBackups.mockRejectedValueOnce(new Error("backup failed")); + + const io = createFastConfigIO(home); + await expect(io.writeConfigFile({ gateway: { mode: "local" } })).rejects.toThrow( + "backup failed", + ); + + const persistedConfig = JSON.parse(await fs.readFile(configPath, "utf-8")) as typeof original; + expect(persistedConfig.plugins.installs.demo).toMatchObject({ + source: "npm", + spec: "demo@1.0.0", + installPath: pluginDir, + }); + await expect( + readPersistedInstalledPluginIndex({ + stateDir: path.join(home, ".openclaw"), + }), + ).resolves.toBeNull(); + }); + }); + const writeGatewayPortAndReadConfig = async (home: string, configPath: string) => { const io = createFastConfigIO(home);