From 7c0f5463a56489a0afcfdd178bc85e8dcf9f4152 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 4 May 2026 14:06:44 -0700 Subject: [PATCH] fix(update): isolate plugin sync failures Disable and skip plugins that fail package-update plugin sync so broken plugin packages do not fail an otherwise successful OpenClaw update. --- CHANGELOG.md | 1 + src/cli/update-cli/update-command.test.ts | 31 ++++ src/cli/update-cli/update-command.ts | 29 +++- src/plugins/update.test.ts | 55 +++++++ src/plugins/update.ts | 170 ++++++++++++---------- 5 files changed, 212 insertions(+), 74 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 655136a6c79..e6c48d21ac4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- CLI/update: disable and skip plugins that fail package-update plugin sync, so a broken npm/ClawHub/git/marketplace plugin cannot turn a successful OpenClaw package update into a failed update result. Thanks @vincentkoc. - Diagnostics: grant the internal diagnostics event bus to official installed diagnostics exporter plugins, so npm-installed `@openclaw/diagnostics-prometheus` can emit metrics without broadening the capability to arbitrary global plugins. Fixes #76628. Thanks @RayWoo. - Browser: enforce strict SSRF current-URL checks before existing-session screenshots, matching existing-session snapshot handling. Thanks @vincentkoc. - Active Memory: give timeout partial transcript recovery enough abort-settle headroom so temporary recall summaries are returned before cleanup. Thanks @vincentkoc. diff --git a/src/cli/update-cli/update-command.test.ts b/src/cli/update-cli/update-command.test.ts index f851bfa2c17..22d0a81333c 100644 --- a/src/cli/update-cli/update-command.test.ts +++ b/src/cli/update-cli/update-command.test.ts @@ -222,6 +222,37 @@ describe("collectMissingPluginInstallPayloads", () => { await fs.rm(tmpDir, { recursive: true, force: true }); } }); + + it("skips disabled tracked records when requested", async () => { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-update-plugin-payload-")); + const missingDir = path.join(tmpDir, "state", "npm", "node_modules", "@openclaw", "missing"); + try { + await expect( + collectMissingPluginInstallPayloads({ + env: { HOME: tmpDir } as NodeJS.ProcessEnv, + skipDisabledPlugins: true, + config: { + plugins: { + entries: { + missing: { + enabled: false, + }, + }, + }, + }, + records: { + missing: { + source: "npm", + spec: "@openclaw/missing@beta", + installPath: missingDir, + }, + }, + }), + ).resolves.toEqual([]); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } + }); }); describe("shouldUseLegacyProcessRestartAfterUpdate", () => { diff --git a/src/cli/update-cli/update-command.ts b/src/cli/update-cli/update-command.ts index 6839f0d7895..1a5d5678ffd 100644 --- a/src/cli/update-cli/update-command.ts +++ b/src/cli/update-cli/update-command.ts @@ -51,6 +51,7 @@ import { resolveGlobalInstallSpec, } from "../../infra/update-global.js"; import { runGatewayUpdate, type UpdateRunResult } from "../../infra/update-runner.js"; +import { normalizePluginsConfig, resolveEffectiveEnableState } from "../../plugins/config-state.js"; import { loadInstalledPluginIndexInstallRecords, withoutPluginInstallRecords, @@ -184,9 +185,15 @@ async function pathExists(filePath: string): Promise { export async function collectMissingPluginInstallPayloads(params: { records: Record; + config?: OpenClawConfig; + skipDisabledPlugins?: boolean; env?: NodeJS.ProcessEnv; }): Promise { const env = params.env ?? process.env; + const normalizedPluginConfig = + params.skipDisabledPlugins && params.config + ? normalizePluginsConfig(params.config.plugins) + : undefined; const missing: MissingPluginInstallPayload[] = []; for (const [pluginId, record] of Object.entries(params.records).toSorted(([left], [right]) => left.localeCompare(right), @@ -194,6 +201,17 @@ export async function collectMissingPluginInstallPayloads(params: { if (!isTrackedPackageInstallRecord(record)) { continue; } + if (normalizedPluginConfig && params.config) { + const enableState = resolveEffectiveEnableState({ + id: pluginId, + origin: "global", + config: normalizedPluginConfig, + rootConfig: params.config, + }); + if (!enableState.enabled) { + continue; + } + } const rawInstallPath = normalizeOptionalString(record.installPath); if (!rawInstallPath) { missing.push({ pluginId, reason: "missing-install-path" }); @@ -1091,7 +1109,11 @@ async function updatePluginsAfterCoreUpdate(params: { const repairMissingPayloads = async ( records: Record, ): Promise => { - const missing = await collectMissingPluginInstallPayloads({ records }); + const missing = await collectMissingPluginInstallPayloads({ + records, + config: pluginConfig, + skipDisabledPlugins: true, + }); if (missing.length === 0) { return []; } @@ -1110,6 +1132,8 @@ async function updatePluginsAfterCoreUpdate(params: { pluginIds: missingIds, timeoutMs: params.timeoutMs, updateChannel: params.channel, + skipDisabledPlugins: true, + disableOnFailure: true, logger: pluginLogger, onIntegrityDrift: onPluginIntegrityDrift, }); @@ -1130,6 +1154,7 @@ async function updatePluginsAfterCoreUpdate(params: { updateChannel: params.channel, skipIds: new Set([...syncResult.summary.switchedToNpm, ...repairedMissingPayloadIds]), skipDisabledPlugins: true, + disableOnFailure: true, logger: pluginLogger, onIntegrityDrift: onPluginIntegrityDrift, }); @@ -1140,6 +1165,8 @@ async function updatePluginsAfterCoreUpdate(params: { const remainingMissingPayloads = await collectMissingPluginInstallPayloads({ records: pluginConfig.plugins?.installs ?? {}, + config: pluginConfig, + skipDisabledPlugins: true, }); pluginUpdateOutcomes.push( ...remainingMissingPayloads.map( diff --git a/src/plugins/update.test.ts b/src/plugins/update.test.ts index 91f9028448a..655d387e32b 100644 --- a/src/plugins/update.test.ts +++ b/src/plugins/update.test.ts @@ -1032,6 +1032,61 @@ describe("updateNpmInstalledPlugins", () => { ]); }); + it("disables enabled tracked plugin update failures when requested", async () => { + const warn = vi.fn(); + installPluginFromNpmSpecMock.mockResolvedValue({ + ok: false, + error: "registry timeout", + }); + const config = { + plugins: { + entries: { + demo: { + enabled: true, + config: { preserved: true }, + }, + }, + installs: { + demo: { + source: "npm" as const, + spec: "@acme/demo", + installPath: "/tmp/demo", + }, + }, + }, + } satisfies OpenClawConfig; + + const result = await updateNpmInstalledPlugins({ + config, + skipDisabledPlugins: true, + disableOnFailure: true, + logger: { warn }, + }); + + expect(installPluginFromNpmSpecMock).toHaveBeenCalledWith( + expect.objectContaining({ + spec: "@acme/demo", + expectedPluginId: "demo", + }), + ); + const message = + 'Disabled "demo" after plugin update failure; OpenClaw will continue without it. Failed to update demo: registry timeout'; + expect(warn).toHaveBeenCalledWith(message); + expect(result.changed).toBe(true); + expect(result.config.plugins?.entries?.demo).toEqual({ + enabled: false, + config: { preserved: true }, + }); + expect(result.config.plugins?.installs?.demo).toEqual(config.plugins.installs.demo); + expect(result.outcomes).toEqual([ + { + pluginId: "demo", + status: "skipped", + message, + }, + ]); + }); + it("aborts exact pinned npm plugin updates on integrity drift by default", async () => { const warn = vi.fn(); installPluginFromNpmSpecMock.mockImplementation( diff --git a/src/plugins/update.ts b/src/plugins/update.ts index 2ca5508ab84..c3b97338c4f 100644 --- a/src/plugins/update.ts +++ b/src/plugins/update.ts @@ -747,12 +747,30 @@ function createPluginUpdateIntegrityDriftHandler(params: { }; } +function disablePluginConfigEntry(config: OpenClawConfig, pluginId: string): OpenClawConfig { + const existingEntry = config.plugins?.entries?.[pluginId]; + return { + ...config, + plugins: { + ...config.plugins, + entries: { + ...config.plugins?.entries, + [pluginId]: { + ...existingEntry, + enabled: false, + }, + }, + }, + }; +} + export async function updateNpmInstalledPlugins(params: { config: OpenClawConfig; logger?: PluginUpdateLogger; pluginIds?: string[]; skipIds?: Set; skipDisabledPlugins?: boolean; + disableOnFailure?: boolean; timeoutMs?: number; dryRun?: boolean; updateChannel?: UpdateChannel; @@ -771,6 +789,28 @@ export async function updateNpmInstalledPlugins(params: { let next = params.config; let changed = false; + const recordFailure = (pluginId: string, message: string) => { + if (params.disableOnFailure && !params.dryRun) { + const disabledMessage = + `Disabled "${pluginId}" after plugin update failure; OpenClaw will continue without it. ` + + message; + logger.warn?.(disabledMessage); + next = disablePluginConfigEntry(next, pluginId); + changed = true; + outcomes.push({ + pluginId, + status: "skipped", + message: disabledMessage, + }); + return; + } + outcomes.push({ + pluginId, + status: "error", + message, + }); + }; + for (const pluginId of targets) { if (params.skipIds?.has(pluginId)) { outcomes.push({ @@ -928,11 +968,7 @@ export async function updateNpmInstalledPlugins(params: { record.installPath?.trim() || resolvePluginInstallDir(pluginId), ); } catch (err) { - outcomes.push({ - pluginId, - status: "error", - message: `Invalid install path for "${pluginId}": ${String(err)}`, - }); + recordFailure(pluginId, `Invalid install path for "${pluginId}": ${String(err)}`); continue; } const currentVersion = await readInstalledPackageVersion(installPath); @@ -1037,11 +1073,7 @@ export async function updateNpmInstalledPlugins(params: { logger, }); } catch (err) { - outcomes.push({ - pluginId, - status: "error", - message: `Failed to check ${pluginId}: ${String(err)}`, - }); + recordFailure(pluginId, `Failed to check ${pluginId}: ${String(err)}`); continue; } let usedNpmFallback = false; @@ -1096,43 +1128,41 @@ export async function updateNpmInstalledPlugins(params: { }); } if (!probe.ok) { - outcomes.push({ + recordFailure( pluginId, - status: "error", - message: - record.source === "npm" - ? formatNpmInstallFailure({ + record.source === "npm" + ? formatNpmInstallFailure({ + pluginId, + spec: npmUpdateFailureSpec({ + effectiveSpec, + fallbackSpec: npmSpecs?.fallbackSpec, + usedFallback: usedNpmFallback, + }), + phase: "check", + result: probe, + }) + : record.source === "clawhub" + ? formatClawHubInstallFailure({ pluginId, - spec: npmUpdateFailureSpec({ - effectiveSpec, - fallbackSpec: npmSpecs?.fallbackSpec, - usedFallback: usedNpmFallback, - }), + spec: effectiveSpec ?? `clawhub:${record.clawhubPackage!}`, phase: "check", - result: probe, + error: probe.error, }) - : record.source === "clawhub" - ? formatClawHubInstallFailure({ + : record.source === "git" + ? formatGitInstallFailure({ pluginId, - spec: effectiveSpec ?? `clawhub:${record.clawhubPackage!}`, + spec: effectiveSpec!, phase: "check", error: probe.error, }) - : record.source === "git" - ? formatGitInstallFailure({ - pluginId, - spec: effectiveSpec!, - phase: "check", - error: probe.error, - }) - : formatMarketplaceInstallFailure({ - pluginId, - marketplaceSource: record.marketplaceSource!, - marketplacePlugin: record.marketplacePlugin!, - phase: "check", - error: probe.error, - }), - }); + : formatMarketplaceInstallFailure({ + pluginId, + marketplaceSource: record.marketplaceSource!, + marketplacePlugin: record.marketplacePlugin!, + phase: "check", + error: probe.error, + }), + ); continue; } @@ -1224,11 +1254,7 @@ export async function updateNpmInstalledPlugins(params: { logger, }); } catch (err) { - outcomes.push({ - pluginId, - status: "error", - message: `Failed to update ${pluginId}: ${String(err)}`, - }); + recordFailure(pluginId, `Failed to update ${pluginId}: ${String(err)}`); continue; } let usedNpmFallback = false; @@ -1281,43 +1307,41 @@ export async function updateNpmInstalledPlugins(params: { }); } if (!result.ok) { - outcomes.push({ + recordFailure( pluginId, - status: "error", - message: - record.source === "npm" - ? formatNpmInstallFailure({ + record.source === "npm" + ? formatNpmInstallFailure({ + pluginId, + spec: npmUpdateFailureSpec({ + effectiveSpec, + fallbackSpec: npmSpecs?.fallbackSpec, + usedFallback: usedNpmFallback, + }), + phase: "update", + result: result, + }) + : record.source === "clawhub" + ? formatClawHubInstallFailure({ pluginId, - spec: npmUpdateFailureSpec({ - effectiveSpec, - fallbackSpec: npmSpecs?.fallbackSpec, - usedFallback: usedNpmFallback, - }), + spec: effectiveSpec ?? `clawhub:${record.clawhubPackage!}`, phase: "update", - result: result, + error: result.error, }) - : record.source === "clawhub" - ? formatClawHubInstallFailure({ + : record.source === "git" + ? formatGitInstallFailure({ pluginId, - spec: effectiveSpec ?? `clawhub:${record.clawhubPackage!}`, + spec: effectiveSpec!, phase: "update", error: result.error, }) - : record.source === "git" - ? formatGitInstallFailure({ - pluginId, - spec: effectiveSpec!, - phase: "update", - error: result.error, - }) - : formatMarketplaceInstallFailure({ - pluginId, - marketplaceSource: record.marketplaceSource!, - marketplacePlugin: record.marketplacePlugin!, - phase: "update", - error: result.error, - }), - }); + : formatMarketplaceInstallFailure({ + pluginId, + marketplaceSource: record.marketplaceSource!, + marketplacePlugin: record.marketplacePlugin!, + phase: "update", + error: result.error, + }), + ); continue; }