mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 09:20:43 +00:00
fix: make plugin install config migration atomic
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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<string>();
|
||||
|
||||
@@ -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<ReadConfigFileSnapshotInternalResult> {
|
||||
async function readConfigFileSnapshotInternal(
|
||||
options: { persistShippedPluginInstallMigration?: boolean } = {},
|
||||
): Promise<ReadConfigFileSnapshotInternalResult> {
|
||||
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<ReadConfigFileSnapshotForWriteResult> {
|
||||
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<string, string> | null = null;
|
||||
let changedPaths: Set<string> | 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;
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user