From a434133aacb18468ebcaf14cd560426f9c637f3d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 26 Apr 2026 09:00:58 +0100 Subject: [PATCH] fix: fail update on plugin sync errors --- CHANGELOG.md | 1 + docs/cli/update.md | 13 +++-- src/cli/update-cli.test.ts | 87 +++++++++++++++++++++++++++- src/cli/update-cli/update-command.ts | 44 +++++++++++--- src/plugins/update.test.ts | 55 ++++++++++++++++++ src/plugins/update.ts | 30 +++++++--- 6 files changed, 207 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c848866dafb..3b5cc9a6bab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,7 @@ Docs: https://docs.openclaw.ai - Windows install/Lobster: execute `pnpm.exe` directly when `npm_execpath` points at the native pnpm binary, add an installed-package fallback for the Lobster embedded runtime, and include the Lobster runner regression test in Windows CI. Fixes #69456. Thanks @igormf. - Gateway/install: refresh loaded gateway service installs when the current service embeds stale gateway auth instead of returning already-installed, avoiding LaunchAgent token-mismatch loops after token rotation. Fixes #70752. Thanks @hyspacex. - Update: ignore bundled plugin `.openclaw-install-stage` directories during global install verification and packaged dist pruning so leftover runtime-dep staging files do not turn successful updates into `unexpected packaged dist file` failures. Fixes #71752. Thanks @waynegault. +- CLI/update: fail package updates when post-update plugin sync fails and refresh legacy npm plugin install records before trusting unchanged artifacts, preventing successful updates from restarting with stale or failed plugin state. Thanks @vincentkoc and @shakkernerd. - Node runtime: keep node-host retry timers alive across Gateway restarts and exit on terminal credential pauses so supervised nodes do not become silent zombies. Fixes #69800. Thanks @meroli28. - Gateway/plugins: stop persisted WhatsApp auth state from activating bundled channel runtime-dependency repair during startup when `channels.whatsapp` is absent, avoiding npm/git stalls on packaged Linux installs. Fixes #71994. Thanks @xiao398008. - Gateway/device tokens: enforce caller-scope containment inside token rotation and revocation so pairing-only sessions cannot mutate higher-scope operator tokens. Fixes #71990. Thanks @coygeek. diff --git a/docs/cli/update.md b/docs/cli/update.md index 1c998ea1d19..e05e4315642 100644 --- a/docs/cli/update.md +++ b/docs/cli/update.md @@ -83,10 +83,11 @@ install method aligned: The Gateway core auto-updater (when enabled via config) reuses this same update path. For package-manager installs, `openclaw update` resolves the target package -version before invoking the package manager. If the installed version exactly -matches the target and no update-channel change needs to be persisted, the -command exits as skipped before package install, plugin sync, completion refresh, -or gateway restart work. +version before invoking the package manager. Even when the installed version +already matches the target, the command refreshes the global package install, +then runs plugin sync, completion refresh, and restart work. This keeps packaged +sidecars and channel-owned plugin records aligned with the installed OpenClaw +build. ## Git checkout flow @@ -114,6 +115,10 @@ differs from the stored install record, `openclaw update` aborts that plugin artifact update instead of installing it. Reinstall or update the plugin explicitly only after verifying that you trust the new artifact. +Post-update plugin sync failures fail the update result and stop restart +follow-up work. Fix the plugin install/update error, then rerun +`openclaw update`. + If pnpm bootstrap still fails, the updater now stops early with a package-manager-specific error instead of trying `npm run build` inside the checkout. ## `--update` shorthand diff --git a/src/cli/update-cli.test.ts b/src/cli/update-cli.test.ts index fe91cb3211f..97887259237 100644 --- a/src/cli/update-cli.test.ts +++ b/src/cli/update-cli.test.ts @@ -667,7 +667,7 @@ describe("update-cli", () => { expect(logs.join("\n")).toContain("Plugin update aborted"); }); - it("includes plugin integrity drift details in update json output", async () => { + it("fails json update output when post-core plugin updates fail", async () => { updateNpmInstalledPlugins.mockImplementationOnce( async (params: { config: OpenClawConfig; @@ -713,6 +713,9 @@ describe("update-cli", () => { const jsonOutput = vi.mocked(defaultRuntime.writeJson).mock.calls.at(-1)?.[0] as | UpdateRunResult | undefined; + expect(defaultRuntime.exit).toHaveBeenCalledWith(1); + expect(jsonOutput?.status).toBe("error"); + expect(jsonOutput?.reason).toBe("post-update-plugins"); expect(jsonOutput?.postUpdate?.plugins?.integrityDrifts).toEqual([ { pluginId: "demo", @@ -728,6 +731,88 @@ describe("update-cli", () => { expect(jsonOutput?.postUpdate?.plugins?.npm.outcomes[0]?.status).toBe("error"); }); + it("fails before restart when post-core plugin updates fail", async () => { + updateNpmInstalledPlugins.mockResolvedValueOnce({ + changed: false, + config: baseConfig, + outcomes: [ + { + pluginId: "demo", + status: "error", + message: "Failed to update demo: registry timeout", + }, + ], + }); + serviceLoaded.mockResolvedValue(true); + + await updateCommand({ yes: true }); + + expect(defaultRuntime.exit).toHaveBeenCalledWith(1); + expect(runDaemonInstall).not.toHaveBeenCalled(); + expect(runDaemonRestart).not.toHaveBeenCalled(); + expect(runRestartScript).not.toHaveBeenCalled(); + expect( + vi + .mocked(defaultRuntime.error) + .mock.calls.map((call) => String(call[0])) + .join("\n"), + ).toContain("Update failed during plugin post-update sync."); + }); + + it("preserves fresh-process plugin failure details in parent json output", async () => { + setupUpdatedRootRefresh(); + spawn.mockImplementationOnce((_node, _argv, options) => { + const child = new EventEmitter() as EventEmitter & { + once: EventEmitter["once"]; + }; + const env = (options as { env?: NodeJS.ProcessEnv }).env; + queueMicrotask(async () => { + const resultPath = env?.OPENCLAW_UPDATE_POST_CORE_RESULT_PATH; + if (resultPath) { + await fs.writeFile( + resultPath, + JSON.stringify({ + status: "error", + changed: false, + sync: { + changed: false, + switchedToBundled: [], + switchedToNpm: [], + warnings: [], + errors: [], + }, + npm: { + changed: false, + outcomes: [ + { + pluginId: "demo", + status: "error", + message: "Failed to update demo: registry timeout", + }, + ], + }, + integrityDrifts: [], + }), + "utf-8", + ); + } + child.emit("exit", 1, null); + }); + return child; + }); + vi.mocked(defaultRuntime.writeJson).mockClear(); + + await updateCommand({ yes: true, json: true, restart: false }); + + const jsonOutput = vi.mocked(defaultRuntime.writeJson).mock.calls.at(-1)?.[0] as + | UpdateRunResult + | undefined; + expect(defaultRuntime.exit).toHaveBeenCalledWith(1); + expect(jsonOutput?.status).toBe("error"); + expect(jsonOutput?.reason).toBe("post-update-plugins"); + expect(jsonOutput?.postUpdate?.plugins?.npm.outcomes[0]?.message).toContain("registry timeout"); + }); + it.each([ { name: "preview mode", diff --git a/src/cli/update-cli/update-command.ts b/src/cli/update-cli/update-command.ts index e9a583fb81c..19537de54ce 100644 --- a/src/cli/update-cli/update-command.ts +++ b/src/cli/update-cli/update-command.ts @@ -1006,13 +1006,16 @@ async function continuePostCoreUpdateInFreshProcess(params: { }); }); - if (exitCode !== 0) { - defaultRuntime.exit(exitCode); - throw new Error(`post-update process exited with code ${exitCode}`); - } const pluginUpdate = resultPath ? await readPostCorePluginUpdateResultFile(resultPath) : undefined; + if (exitCode !== 0) { + if (pluginUpdate) { + return { resumed: true, pluginUpdate }; + } + defaultRuntime.exit(exitCode); + throw new Error(`post-update process exited with code ${exitCode}`); + } return { resumed: true, ...(pluginUpdate ? { pluginUpdate } : {}) }; } finally { if (resultDir) { @@ -1075,6 +1078,10 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise { defaultRuntime.writeJson(result); } } + if (pluginUpdate.status === "error") { + defaultRuntime.exit(1); + return; + } return; } @@ -1434,6 +1441,28 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise { }); } + const resultWithPostUpdate: UpdateRunResult = postCorePluginUpdate + ? { + ...result, + status: postCorePluginUpdate.status === "error" ? "error" : result.status, + ...(postCorePluginUpdate.status === "error" ? { reason: "post-update-plugins" } : {}), + postUpdate: { + ...result.postUpdate, + plugins: postCorePluginUpdate, + }, + } + : result; + + if (postCorePluginUpdate?.status === "error") { + if (opts.json) { + defaultRuntime.writeJson(resultWithPostUpdate); + } else { + defaultRuntime.error(theme.error("Update failed during plugin post-update sync.")); + } + defaultRuntime.exit(1); + return; + } + let restartScriptPath: string | null = null; let refreshGatewayServiceEnv = false; const gatewayPort = resolveGatewayPort( @@ -1469,7 +1498,7 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise { const restartOk = await maybeRestartService({ shouldRestart, - result, + result: resultWithPostUpdate, opts, refreshServiceEnv: refreshGatewayServiceEnv, gatewayPort, @@ -1485,9 +1514,6 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise { if (!opts.json) { defaultRuntime.log(theme.muted(pickUpdateQuip())); } else { - defaultRuntime.writeJson({ - ...result, - ...(postCorePluginUpdate ? { postUpdate: { plugins: postCorePluginUpdate } } : {}), - }); + defaultRuntime.writeJson(resultWithPostUpdate); } } diff --git a/src/plugins/update.test.ts b/src/plugins/update.test.ts index adc2af7be2c..01be9f10307 100644 --- a/src/plugins/update.test.ts +++ b/src/plugins/update.test.ts @@ -71,8 +71,10 @@ function createNpmInstallConfig(params: { spec: string; installPath: string; integrity?: string; + shasum?: string; resolvedName?: string; resolvedSpec?: string; + resolvedVersion?: string; }) { return { plugins: { @@ -82,8 +84,10 @@ function createNpmInstallConfig(params: { spec: params.spec, installPath: params.installPath, ...(params.integrity ? { integrity: params.integrity } : {}), + ...(params.shasum ? { shasum: params.shasum } : {}), ...(params.resolvedName ? { resolvedName: params.resolvedName } : {}), ...(params.resolvedSpec ? { resolvedSpec: params.resolvedSpec } : {}), + ...(params.resolvedVersion ? { resolvedVersion: params.resolvedVersion } : {}), }, }, }, @@ -412,6 +416,55 @@ describe("updateNpmInstalledPlugins", () => { ]); }); + it("refreshes legacy npm install records before skipping unchanged artifacts", async () => { + const installPath = createInstalledPackageDir({ + name: "@martian-engineering/lossless-claw", + version: "0.9.0", + }); + mockNpmViewMetadata({ + name: "@martian-engineering/lossless-claw", + version: "0.9.0", + integrity: "sha512-same", + shasum: "same", + }); + installPluginFromNpmSpecMock.mockResolvedValue( + createSuccessfulNpmUpdateResult({ + pluginId: "lossless-claw", + targetDir: installPath, + version: "0.9.0", + npmResolution: { + name: "@martian-engineering/lossless-claw", + version: "0.9.0", + resolvedSpec: "@martian-engineering/lossless-claw@0.9.0", + }, + }), + ); + + const result = await updateNpmInstalledPlugins({ + config: createNpmInstallConfig({ + pluginId: "lossless-claw", + spec: "@martian-engineering/lossless-claw", + installPath, + }), + pluginIds: ["lossless-claw"], + }); + + expect(installPluginFromNpmSpecMock).toHaveBeenCalledTimes(1); + expect(result.changed).toBe(true); + expect(result.outcomes[0]).toMatchObject({ + pluginId: "lossless-claw", + status: "unchanged", + currentVersion: "0.9.0", + nextVersion: "0.9.0", + }); + expect(result.config.plugins?.installs?.["lossless-claw"]).toMatchObject({ + source: "npm", + resolvedName: "@martian-engineering/lossless-claw", + resolvedVersion: "0.9.0", + resolvedSpec: "@martian-engineering/lossless-claw@0.9.0", + }); + }); + it("expands home-relative install paths before checking installed npm versions", async () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-plugin-update-home-")); tempDirs.push(home); @@ -436,8 +489,10 @@ describe("updateNpmInstalledPlugins", () => { spec: "@martian-engineering/lossless-claw", installPath: "~/.openclaw/extensions/lossless-claw", resolvedName: "@martian-engineering/lossless-claw", + resolvedVersion: "0.9.0", resolvedSpec: "@martian-engineering/lossless-claw@0.9.0", integrity: "sha512-same", + shasum: "same", }), pluginIds: ["lossless-claw"], }); diff --git a/src/plugins/update.ts b/src/plugins/update.ts index 48da2a3bea1..32e74c051ef 100644 --- a/src/plugins/update.ts +++ b/src/plugins/update.ts @@ -115,10 +115,6 @@ type InstallIntegrityDrift = { }; }; -function stringFieldMatches(recorded: string | undefined, resolved: string | undefined): boolean { - return !recorded || (resolved !== undefined && recorded === resolved); -} - function shouldSkipUnchangedNpmInstall(params: { currentVersion?: string; record: { @@ -136,12 +132,28 @@ function shouldSkipUnchangedNpmInstall(params: { if (params.currentVersion !== params.metadata.version) { return false; } + if ( + !params.record.resolvedName || + !params.record.resolvedSpec || + !params.record.resolvedVersion + ) { + return false; + } + if (!params.metadata.name || !params.metadata.resolvedSpec) { + return false; + } + if (params.metadata.integrity && !params.record.integrity) { + return false; + } + if (params.metadata.shasum && !params.record.shasum) { + return false; + } return ( - stringFieldMatches(params.record.integrity, params.metadata.integrity) && - stringFieldMatches(params.record.shasum, params.metadata.shasum) && - stringFieldMatches(params.record.resolvedName, params.metadata.name) && - stringFieldMatches(params.record.resolvedSpec, params.metadata.resolvedSpec) && - stringFieldMatches(params.record.resolvedVersion, params.metadata.version) + (!params.metadata.integrity || params.record.integrity === params.metadata.integrity) && + (!params.metadata.shasum || params.record.shasum === params.metadata.shasum) && + params.record.resolvedName === params.metadata.name && + params.record.resolvedSpec === params.metadata.resolvedSpec && + params.record.resolvedVersion === params.metadata.version ); }