fix(update): preserve plugin policy on failures (#108661)

This commit is contained in:
Peter Steinberger
2026-07-16 00:38:44 -07:00
committed by GitHub
parent 138178620d
commit 7a398fe6f8
6 changed files with 59 additions and 22 deletions
+1 -1
View File
@@ -277,7 +277,7 @@ If an exact pinned npm plugin update resolves to an artifact whose integrity dif
</Warning>
<Note>
Post-update plugin sync failures that are scoped to a managed plugin and that the sync path can route around (for example an unreachable npm registry for a non-essential plugin) are reported as warnings after the core update succeeds. The JSON result keeps top-level update `status: "ok"` and reports `postUpdate.plugins.status: "warning"` with `openclaw update repair` and `openclaw plugins inspect <id> --runtime --json` guidance. Unexpected updater or sync exceptions still fail the update result. Fix the plugin install or update error, then rerun `openclaw update repair`.
Post-update plugin sync failures that are scoped to a managed plugin and that the sync path can route around (for example an unreachable npm registry for a non-essential plugin) are reported as warnings after the core update succeeds. The JSON result keeps top-level update `status: "ok"` and reports `postUpdate.plugins.status: "warning"` with `openclaw update repair` and `openclaw plugins inspect <id> --runtime --json` guidance. Unexpected updater or sync exceptions still fail the update result. Fix the plugin install or update error, then rerun `openclaw update repair`. When a failed update leaves a managed plugin unusable, OpenClaw disables its runtime entry and resets active slots without changing the operator-authored `plugins.allow` or `plugins.deny` policy.
After the per-plugin sync step, `openclaw update` runs a mandatory **post-core convergence** pass before the gateway restarts: it repairs missing configured plugin payloads, validates each _active_ tracked install record on disk, and statically verifies its `package.json` is parseable (and any explicitly declared `main` exists). Failures from this pass, and an invalid config snapshot, return `postUpdate.plugins.status: "error"` and flip the top-level update `status` to `"error"`, so `openclaw update` exits non-zero and the gateway is _not_ restarted with an unverified plugin set. The error includes structured `postUpdate.plugins.warnings[].guidance` lines pointing at `openclaw update repair` and `openclaw plugins inspect <id> --runtime --json`. Disabled plugin entries and records that are not trusted-source-linked official sync targets are skipped here (mirroring the `skipDisabledPlugins` policy used by the missing-payload check), so a stale disabled plugin record cannot block an otherwise valid update.
@@ -41,6 +41,7 @@ start_npm_fixture_registry "@openclaw/demo-corrupt-plugin" "0.0.1" /tmp/demo-cor
echo "Installing managed external plugin..."
node "$entry" plugins install "npm:@openclaw/demo-corrupt-plugin@0.0.1" --force >/tmp/openclaw-corrupt-plugin-install.log 2>&1
node "$entry" config set plugins.allow '["demo-corrupt-plugin"]' >/dev/null
node "$entry" plugins inspect demo-corrupt-plugin --runtime --json >/tmp/openclaw-corrupt-plugin-before.json
unset NPM_CONFIG_REGISTRY npm_config_registry
@@ -105,6 +106,7 @@ if [ "$update_status" -ne 0 ]; then
exit "$post_core_status"
fi
node scripts/e2e/lib/plugin-update/probe.mjs assert-corrupt-plugin-result /tmp/openclaw-update-corrupt-plugin-post-core.json demo-corrupt-plugin
node scripts/e2e/lib/plugin-update/probe.mjs assert-disabled-policy-preserved "$OPENCLAW_CONFIG_PATH" demo-corrupt-plugin
exit 0
fi
@@ -115,3 +117,4 @@ if ! node scripts/e2e/lib/plugin-update/probe.mjs assert-corrupt-update /tmp/ope
openclaw_e2e_print_log /tmp/openclaw-update-corrupt-plugin.err >&2
exit 1
fi
node scripts/e2e/lib/plugin-update/probe.mjs assert-disabled-policy-preserved "$OPENCLAW_CONFIG_PATH" demo-corrupt-plugin
+12
View File
@@ -286,6 +286,17 @@ function assertLegacyPostUpdatePluginFailure(updateJsonPath) {
}
}
function assertDisabledPluginPolicyPreserved(configPath, pluginId) {
const config = readJson(configPath);
const allow = config.plugins?.allow;
if (JSON.stringify(allow) !== JSON.stringify([pluginId])) {
throw new Error(`expected plugins.allow to preserve ${pluginId}, got ${JSON.stringify(allow)}`);
}
if (config.plugins?.entries?.[pluginId]?.enabled !== false) {
throw new Error(`expected ${pluginId} to be disabled after update failure`);
}
}
const [command, arg, arg2] = process.argv.slice(2);
const commands = {
"legacy-compat": () => console.log(legacyPackageAcceptanceCompat(arg || "") ? "1" : "0"),
@@ -297,6 +308,7 @@ const commands = {
"assert-corrupt-update": () => assertCorruptUpdate(arg, arg2),
"assert-corrupt-plugin-result": () => assertCorruptPluginResult(arg, arg2),
"assert-legacy-post-update-plugin-failure": () => assertLegacyPostUpdatePluginFailure(arg),
"assert-disabled-policy-preserved": () => assertDisabledPluginPolicyPreserved(arg, arg2),
};
const run = commands[command];
await (
+18 -6
View File
@@ -2173,6 +2173,11 @@ describe("updateNpmInstalledPlugins", () => {
config: {
plugins: {
allow: ["lossless-claw", "keep"],
deny: ["lossless-claw", "blocked"],
slots: {
memory: "lossless-claw",
contextEngine: "lossless-claw",
},
entries: {
"lossless-claw": {
enabled: true,
@@ -2201,7 +2206,12 @@ describe("updateNpmInstalledPlugins", () => {
enabled: false,
config: { preserved: true },
});
expect(result.config.plugins?.allow).toEqual(["keep"]);
expect(result.config.plugins?.allow).toEqual(["lossless-claw", "keep"]);
expect(result.config.plugins?.deny).toEqual(["lossless-claw", "blocked"]);
expect(result.config.plugins?.slots).toEqual({
memory: "memory-core",
contextEngine: "legacy",
});
expect(result.outcomes).toEqual([
{
pluginId: "lossless-claw",
@@ -2265,6 +2275,7 @@ describe("updateNpmInstalledPlugins", () => {
const config = {
plugins: {
allow: ["demo", "other"],
deny: ["demo", "blocked"],
slots: { memory: "demo" },
entries: {
demo: {
@@ -2297,7 +2308,8 @@ describe("updateNpmInstalledPlugins", () => {
enabled: false,
config: { preserved: true },
});
expect(result.config.plugins?.allow).toEqual(["other"]);
expect(result.config.plugins?.allow).toEqual(["demo", "other"]);
expect(result.config.plugins?.deny).toEqual(["demo", "blocked"]);
expect(result.config.plugins?.slots?.memory).toBe("memory-core");
expect(result.outcomes).toEqual([
{
@@ -2883,7 +2895,7 @@ describe("updateNpmInstalledPlugins", () => {
]);
});
it("disables enabled tracked plugin update failures when requested", async () => {
it("disables failed plugin activation without revoking explicit policy", async () => {
const warn = vi.fn();
installPluginFromNpmSpecMock.mockResolvedValue({
ok: false,
@@ -2930,7 +2942,7 @@ describe("updateNpmInstalledPlugins", () => {
enabled: false,
config: { preserved: true },
});
expect(result.config.plugins?.allow).toEqual(["other"]);
expect(result.config.plugins?.allow).toEqual(["demo", "other"]);
expect(result.config.plugins?.deny).toEqual(["blocked"]);
expect(result.config.plugins?.slots).toEqual({
memory: "memory-core",
@@ -3033,7 +3045,7 @@ describe("updateNpmInstalledPlugins", () => {
enabled: false,
config: { preserved: true },
});
expect(result.config.plugins?.allow).toBeUndefined();
expect(result.config.plugins?.allow).toEqual(["demo"]);
expect(result.config.plugins?.slots?.memory).toBe("memory-core");
expect(result.outcomes).toEqual([
{
@@ -3182,7 +3194,7 @@ describe("updateNpmInstalledPlugins", () => {
enabled: false,
config: { preserved: true },
});
expect(result.config.plugins?.allow).toBeUndefined();
expect(result.config.plugins?.allow).toEqual(["demo"]);
expect(result.config.plugins?.slots).toEqual({
memory: "memory-core",
});
+4 -15
View File
@@ -1287,17 +1287,6 @@ function createPluginUpdateIntegrityDriftHandler(params: {
};
}
function removeDisabledPluginIdFromList(
list: string[] | undefined,
pluginId: string,
): string[] | undefined {
if (!Array.isArray(list) || !list.includes(pluginId)) {
return list;
}
const next = list.filter((id) => id !== pluginId);
return next.length > 0 ? next : undefined;
}
function resetDisabledPluginSlots(
slots: NonNullable<OpenClawConfig["plugins"]>["slots"] | undefined,
pluginId: string,
@@ -1321,15 +1310,15 @@ function resetDisabledPluginSlots(
return next;
}
function disablePluginConfigEntry(config: OpenClawConfig, pluginId: string): OpenClawConfig {
function disablePluginAfterUpdateFailure(config: OpenClawConfig, pluginId: string): OpenClawConfig {
const pluginsConfig = config.plugins ?? {};
const existingEntry = pluginsConfig.entries?.[pluginId];
return {
...config,
plugins: {
...pluginsConfig,
allow: removeDisabledPluginIdFromList(pluginsConfig.allow, pluginId),
deny: removeDisabledPluginIdFromList(pluginsConfig.deny, pluginId),
// Update failure changes activation, not operator-authored trust policy.
// Removing the final allow entry would widen discovery to other plugins.
slots: resetDisabledPluginSlots(pluginsConfig.slots, pluginId),
entries: {
...pluginsConfig.entries,
@@ -1459,7 +1448,7 @@ export async function updateNpmInstalledPlugins(params: {
`Disabled "${pluginId}" after plugin update failure; OpenClaw will continue without it. ` +
message;
logger.warn?.(disabledMessage);
next = disablePluginConfigEntry(next, pluginId);
next = disablePluginAfterUpdateFailure(next, pluginId);
changed = true;
outcomes.push({
pluginId,
@@ -202,6 +202,7 @@ describe("plugin update unchanged Docker E2E", () => {
const script = readFileSync(CORRUPT_UPDATE_SCENARIO_SCRIPT, "utf8");
expect(script).toContain('plugins install "npm:@openclaw/demo-corrupt-plugin@0.0.1" --force');
expect(script).toContain("config set plugins.allow '[\"demo-corrupt-plugin\"]'");
expect(script).toContain("OPENCLAW_UPDATE_CORRUPT_PLUGIN_TIMEOUT_SECONDS");
expect(script).toContain(
"openclaw_e2e_read_positive_int_env OPENCLAW_UPDATE_CORRUPT_PLUGIN_TIMEOUT_SECONDS 900",
@@ -230,6 +231,26 @@ describe("plugin update unchanged Docker E2E", () => {
);
expect(script.match(/openclaw_e2e_print_log \/tmp\/openclaw-update-corrupt-/g)).toHaveLength(8);
expect(script).not.toContain("cat /tmp/openclaw-update-corrupt-");
expect(script.match(/assert-disabled-policy-preserved/g)).toHaveLength(2);
});
it("requires corrupt update failures to preserve the explicit allow policy", () => {
expect(() =>
runProbe("assert-disabled-policy-preserved", {
plugins: {
allow: [CORRUPT_PLUGIN_ID],
entries: { [CORRUPT_PLUGIN_ID]: { enabled: false } },
},
}),
).not.toThrow();
const revokedPolicy = runProbeStatus("assert-disabled-policy-preserved", {
plugins: {
entries: { [CORRUPT_PLUGIN_ID]: { enabled: false } },
},
});
expect(revokedPolicy.status).not.toBe(0);
expect(revokedPolicy.stderr).toContain("expected plugins.allow to preserve");
});
it("requires disabled-after-failure corrupt plugin updates to stay warnings", () => {