From e8885eefcd8865a6e7d4f8b60a848eb10cdd64ca Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Wed, 12 Aug 2026 03:16:22 +0800 Subject: [PATCH] fix(update): migrate plugin config before final validation (#122199) Stage transitional plugin install-record writes without plugin-schema validation, then require fresh doctor and strict validation for every aggregate plugin change. Co-authored-by: Peter Steinberger --- src/cli/update-cli.test.ts | 71 +++++++++++++++++++ src/cli/update-cli/update-command-plugins.ts | 3 + .../update-cli/update-command-post-update.ts | 7 +- src/cli/update-cli/update-command-resume.ts | 3 +- 4 files changed, 77 insertions(+), 7 deletions(-) diff --git a/src/cli/update-cli.test.ts b/src/cli/update-cli.test.ts index 3a0cfd6cec62..74a99dc53e30 100644 --- a/src/cli/update-cli.test.ts +++ b/src/cli/update-cli.test.ts @@ -2410,6 +2410,32 @@ describe("update-cli", () => { expect(defaultRuntime.exit).not.toHaveBeenCalledWith(1); }); + it("runs the final fresh doctor for convergence-only current-process changes", async () => { + mockGitUpdateAfterMutation(); + vi.mocked(resolveGatewayInstallEntrypoint).mockResolvedValueOnce(FRESH_POST_UPDATE_ENTRYPOINT); + runPostCorePluginConvergenceSpy.mockResolvedValueOnce({ + changes: ["Repaired configured plugin install records."], + warnings: [], + errored: false, + smokeFailures: [], + installRecords: {}, + }); + + await updateCommand({ yes: true, restart: false }); + + expect(spawn).not.toHaveBeenCalled(); + const doctorCall = vi.mocked(runExec).mock.calls.find(([, args]) => args[1] === "doctor"); + expect(doctorCall?.[2]).toMatchObject({ + env: { OPENCLAW_UPDATE_POST_CORE_CONVERGENCE: "1" }, + }); + const strictValidationCall = vi + .mocked(runExec) + .mock.calls.find(([, args]) => args[1] === "config" && args[2] === "validate"); + expect(strictValidationCall?.[2]).toMatchObject({ + env: { OPENCLAW_UPDATE_IN_PROGRESS: "0" }, + }); + }); + it("runs the fresh plugin doctor with the selected Node runner", async () => { vi.mocked(resolveGatewayInstallEntrypoint).mockResolvedValueOnce( "/tmp/openclaw-updated-entry.mjs", @@ -2547,6 +2573,51 @@ describe("update-cli", () => { expect(spawn).not.toHaveBeenCalled(); }); + it("stages plugin-changing post-core config before updated plugin migrations run", async () => { + syncPluginsForUpdateChannel.mockImplementationOnce(async ({ config }) => + pluginSyncResult(config, true), + ); + + await runPostCoreCommand({ restart: false }); + + expect(lastReplaceConfigCall()).toMatchObject({ + writeOptions: { skipPluginValidation: true }, + }); + }); + + it("runs the final fresh doctor for convergence-only post-core changes", async () => { + vi.mocked(resolveGatewayInstallEntrypoint).mockResolvedValueOnce(FRESH_POST_UPDATE_ENTRYPOINT); + runPostCorePluginConvergenceSpy.mockResolvedValueOnce({ + changes: ["Repaired configured plugin install records."], + warnings: [], + errored: false, + smokeFailures: [], + installRecords: {}, + }); + + await runPostCoreCommand({ restart: false }); + + expect(syncPluginCall()?.config).toBeDefined(); + expect(updateNpmInstalledPlugins).toHaveBeenCalledTimes(1); + const doctorCalls = vi.mocked(runExec).mock.calls.filter(([, args]) => args[1] === "doctor"); + expect(doctorCalls).toHaveLength(2); + expect(doctorCalls[1]?.[2]).toMatchObject({ + env: { OPENCLAW_UPDATE_POST_CORE_CONVERGENCE: "1" }, + }); + const strictValidationCall = vi + .mocked(runExec) + .mock.calls.find( + ([, args]) => + args[0] === FRESH_POST_UPDATE_ENTRYPOINT && + args[1] === "config" && + args[2] === "validate" && + args[3] === "--json", + ); + expect(strictValidationCall?.[2]).toMatchObject({ + env: { OPENCLAW_UPDATE_IN_PROGRESS: "0" }, + }); + }); + it("keeps fresh doctor output off stdout during json post-core resume", async () => { vi.mocked(runExec).mockResolvedValueOnce({ stdout: "doctor ui output", diff --git a/src/cli/update-cli/update-command-plugins.ts b/src/cli/update-cli/update-command-plugins.ts index 06bcd6dfa4ef..9765c74b0e28 100644 --- a/src/cli/update-cli/update-command-plugins.ts +++ b/src/cli/update-cli/update-command-plugins.ts @@ -476,11 +476,14 @@ export async function updatePluginsAfterCoreUpdate(params: { channels: structuredClone(params.restoredAuthoredChannels) as OpenClawConfig["channels"], }; } + // Installed plugin metadata can own migrations that this process has not loaded yet. + // Finalization runs fresh doctor plus strict validation before the update can complete. await commitPluginInstallRecordsWithConfig({ previousInstallRecords: pluginInstallRecords, nextInstallRecords, nextConfig, baseHash: params.configSnapshot.hash, + writeOptions: { skipPluginValidation: true }, }); await refreshPluginRegistryAfterConfigMutation({ config: nextConfig, diff --git a/src/cli/update-cli/update-command-post-update.ts b/src/cli/update-cli/update-command-post-update.ts index ee348512d32c..e467c58b05b9 100644 --- a/src/cli/update-cli/update-command-post-update.ts +++ b/src/cli/update-cli/update-command-post-update.ts @@ -288,12 +288,9 @@ export async function finishUpdate(params: { const completedPluginUpdate = await completePostCorePluginUpdate({ root: postUpdateRoot, pluginUpdate: initialPluginUpdate, - // A plugin-only update can replace its migration owner without replacing core. - // Downgrades and resume fallbacks can also leave an updated core on disk in this process. + // Aggregate plugin changes and core install changes independently require fresh doctor. freshDoctorRequired: - didCoreUpdateChangeInstall(params.result) || - initialPluginUpdate.sync.changed || - initialPluginUpdate.npm.changed, + didCoreUpdateChangeInstall(params.result) || initialPluginUpdate.changed, yes: params.opts.yes === true, json: params.opts.json === true, timeoutMs: params.updateStepTimeoutMs, diff --git a/src/cli/update-cli/update-command-resume.ts b/src/cli/update-cli/update-command-resume.ts index 44d5d8fe260a..f466a36c2524 100644 --- a/src/cli/update-cli/update-command-resume.ts +++ b/src/cli/update-cli/update-command-resume.ts @@ -127,8 +127,7 @@ async function resumePostCoreUpdateUnlocked(params: ResumePostCoreUpdateParams): const { pluginUpdate } = await completePostCorePluginUpdate({ root: params.root, pluginUpdate: initialPluginUpdate, - // Only package/channel sync can replace the migration owner loaded by this process. - freshDoctorRequired: initialPluginUpdate.sync.changed || initialPluginUpdate.npm.changed, + freshDoctorRequired: initialPluginUpdate.changed, yes: params.opts.yes === true, json: params.opts.json === true, timeoutMs: params.timeoutMs,