From 64ab50e42bad42156e275f5b0f8f02d237458e2c Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 5 May 2026 18:32:09 -0700 Subject: [PATCH] fix(update): preserve plugin warning context --- .../plugin-update/corrupt-update-scenario.sh | 8 +- scripts/e2e/lib/plugin-update/probe.mjs | 10 +- src/cli/update-cli.test.ts | 110 ++++++++++++++++++ src/cli/update-cli/update-command.ts | 62 +++++++++- 4 files changed, 185 insertions(+), 5 deletions(-) diff --git a/scripts/e2e/lib/plugin-update/corrupt-update-scenario.sh b/scripts/e2e/lib/plugin-update/corrupt-update-scenario.sh index 87da071ad95..69e59ef1c58 100644 --- a/scripts/e2e/lib/plugin-update/corrupt-update-scenario.sh +++ b/scripts/e2e/lib/plugin-update/corrupt-update-scenario.sh @@ -86,4 +86,10 @@ if [ "$update_status" -ne 0 ]; then exit 0 fi -node scripts/e2e/lib/plugin-update/probe.mjs assert-corrupt-update /tmp/openclaw-update-corrupt-plugin.json demo-corrupt-plugin +if ! node scripts/e2e/lib/plugin-update/probe.mjs assert-corrupt-update /tmp/openclaw-update-corrupt-plugin.json demo-corrupt-plugin; then + echo "corrupt update JSON payload:" >&2 + cat /tmp/openclaw-update-corrupt-plugin.json >&2 || true + echo "corrupt update stderr:" >&2 + cat /tmp/openclaw-update-corrupt-plugin.err >&2 || true + exit 1 +fi diff --git a/scripts/e2e/lib/plugin-update/probe.mjs b/scripts/e2e/lib/plugin-update/probe.mjs index c77b41099ac..11e001be35b 100644 --- a/scripts/e2e/lib/plugin-update/probe.mjs +++ b/scripts/e2e/lib/plugin-update/probe.mjs @@ -174,9 +174,12 @@ function assertCorruptPluginCleanOrRepaired(evidence) { function assertCorruptPluginDetails(plugins, pluginId) { const evidence = collectPluginEvidence(plugins, pluginId); const outcome = evidence.outcome; - if (!outcome || outcome.status !== "error") { + if ( + !outcome || + (outcome.status !== "error" && !isCorruptPluginDisabledAfterUpdate(evidence, pluginId)) + ) { throw new Error( - `expected error outcome for ${pluginId}, got ${JSON.stringify({ + `expected error or disabled-after-failure outcome for ${pluginId}, got ${JSON.stringify({ outcomes: plugins.npm?.outcomes ?? [], warnings: plugins.warnings ?? [], sync: plugins.sync, @@ -184,6 +187,9 @@ function assertCorruptPluginDetails(plugins, pluginId) { })}`, ); } + if (isCorruptPluginDisabledAfterUpdate(evidence, pluginId)) { + return; + } const warning = evidence.warning; if (!warning) { throw new Error( diff --git a/src/cli/update-cli.test.ts b/src/cli/update-cli.test.ts index 405b471f3a2..39d6e391c62 100644 --- a/src/cli/update-cli.test.ts +++ b/src/cli/update-cli.test.ts @@ -689,6 +689,40 @@ describe("update-cli", () => { expect(spawnEnv?.OPENCLAW_SERVICE_KIND).toBeUndefined(); }); + it("passes pre-update plugin install records into the post-core update process", async () => { + setupUpdatedRootRefresh(); + const pluginInstallRecords = { + demo: { + source: "npm", + spec: "@openclaw/demo@1.0.0", + installPath: "/tmp/openclaw-demo-plugin", + }, + } as const; + let capturedRecords: unknown; + loadInstalledPluginIndexInstallRecords.mockResolvedValueOnce(pluginInstallRecords); + spawn.mockImplementationOnce((_node, _argv, options) => { + const env = (options as { env?: NodeJS.ProcessEnv }).env; + const recordsPath = env?.OPENCLAW_UPDATE_POST_CORE_INSTALL_RECORDS_PATH; + if (!recordsPath) { + throw new Error("missing post-core install records path"); + } + capturedRecords = JSON.parse(fsSync.readFileSync(recordsPath, "utf-8")); + const child = new EventEmitter() as EventEmitter & { + once: EventEmitter["once"]; + }; + queueMicrotask(() => { + child.emit("exit", 0, null); + }); + return child; + }); + + await updateCommand({ yes: true, restart: false }); + + expect(capturedRecords).toEqual(pluginInstallRecords); + expect(syncPluginsForUpdateChannel).not.toHaveBeenCalled(); + expect(updateNpmInstalledPlugins).not.toHaveBeenCalled(); + }); + it("respawns into the updated git root before requested channel persistence", async () => { const { entrypoints } = setupUpdatedRootRefresh({ gatewayUpdateImpl: async (root) => @@ -829,6 +863,48 @@ describe("update-cli", () => { expect(spawn).not.toHaveBeenCalled(); }); + it("post-core resume mode uses the parent install records snapshot for missing payload warnings", async () => { + const resultDir = createCaseDir("openclaw-post-core-records"); + const recordsPath = path.join(resultDir, "plugin-install-records.json"); + const installPath = path.join(resultDir, "demo-plugin"); + await fs.mkdir(installPath, { recursive: true }); + await fs.writeFile( + recordsPath, + `${JSON.stringify({ + demo: { + source: "npm", + spec: "@openclaw/demo@1.0.0", + installPath, + }, + })}\n`, + "utf-8", + ); + pathExists.mockImplementation(async (candidate: string) => candidate === installPath); + + await withEnvAsync( + { + OPENCLAW_UPDATE_POST_CORE: "1", + OPENCLAW_UPDATE_POST_CORE_CHANNEL: "stable", + OPENCLAW_UPDATE_POST_CORE_INSTALL_RECORDS_PATH: recordsPath, + }, + async () => { + await updateCommand({ json: true, restart: false }); + }, + ); + + const jsonOutput = vi.mocked(defaultRuntime.writeJson).mock.calls.at(-1)?.[0] as + | UpdateRunResult + | undefined; + expect(jsonOutput?.postUpdate?.plugins?.status).toBe("warning"); + expect(jsonOutput?.postUpdate?.plugins?.warnings?.[0]?.reason).toContain( + "package.json is missing", + ); + const updateCall = updateNpmInstalledPlugins.mock.calls.at(-1)?.[0] as + | { skipIds?: Set } + | undefined; + expect(updateCall?.skipIds?.has("demo")).toBe(true); + }); + it("post-core resume mode persists the requested update channel with the updated process", async () => { vi.mocked(readConfigFileSnapshot).mockResolvedValue({ ...baseSnapshot, @@ -1175,6 +1251,40 @@ describe("update-cli", () => { expect(logs).toContain("Run openclaw plugins inspect demo --runtime --json for details."); }); + it("marks disabled-after-failure plugin skips as post-update warnings", async () => { + updateNpmInstalledPlugins.mockResolvedValueOnce({ + changed: true, + config: baseConfig, + outcomes: [ + { + pluginId: "demo", + status: "skipped", + message: + 'Disabled "demo" after plugin update failure; OpenClaw will continue without it. Failed to update demo: registry timeout', + }, + ], + }); + vi.mocked(defaultRuntime.writeJson).mockClear(); + + await updateCommand({ json: true, restart: false }); + + const jsonOutput = vi.mocked(defaultRuntime.writeJson).mock.calls.at(-1)?.[0] as + | UpdateRunResult + | undefined; + expect(jsonOutput?.postUpdate?.plugins?.status).toBe("warning"); + expect(jsonOutput?.postUpdate?.plugins?.warnings?.[0]).toMatchObject({ + pluginId: "demo", + guidance: [ + "Run openclaw doctor --fix to attempt automatic repair.", + "Run openclaw plugins inspect demo --runtime --json for details.", + ], + }); + expect(jsonOutput?.postUpdate?.plugins?.npm.outcomes[0]).toMatchObject({ + pluginId: "demo", + status: "skipped", + }); + }); + it("fails unexpected post-core plugin sync exceptions", async () => { syncPluginsForUpdateChannel.mockRejectedValueOnce(new Error("plugin sync invariant broke")); diff --git a/src/cli/update-cli/update-command.ts b/src/cli/update-cli/update-command.ts index 557e6cba192..9126f01f780 100644 --- a/src/cli/update-cli/update-command.ts +++ b/src/cli/update-cli/update-command.ts @@ -115,6 +115,7 @@ const POST_CORE_UPDATE_ENV = "OPENCLAW_UPDATE_POST_CORE"; const POST_CORE_UPDATE_CHANNEL_ENV = "OPENCLAW_UPDATE_POST_CORE_CHANNEL"; const POST_CORE_UPDATE_REQUESTED_CHANNEL_ENV = "OPENCLAW_UPDATE_POST_CORE_REQUESTED_CHANNEL"; const POST_CORE_UPDATE_RESULT_PATH_ENV = "OPENCLAW_UPDATE_POST_CORE_RESULT_PATH"; +const POST_CORE_UPDATE_INSTALL_RECORDS_PATH_ENV = "OPENCLAW_UPDATE_POST_CORE_INSTALL_RECORDS_PATH"; const POST_CORE_UPDATE_RESULT_POLL_MS = 100; const UPDATE_PARENT_SUPPORTS_DOCTOR_CONFIG_WRITE_ENV = "OPENCLAW_UPDATE_PARENT_SUPPORTS_DOCTOR_CONFIG_WRITE"; @@ -181,6 +182,25 @@ function isTrackedPackageInstallRecord(record: PluginInstallRecord): boolean { ); } +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function normalizePluginInstallRecordMap(value: unknown): Record { + if (!isRecord(value)) { + return {}; + } + const records: Record = {}; + for (const [pluginId, record] of Object.entries(value).toSorted(([left], [right]) => + left.localeCompare(right), + )) { + if (isRecord(record) && typeof record.source === "string") { + records[pluginId] = structuredClone(record) as PluginInstallRecord; + } + } + return records; +} + export async function collectMissingPluginInstallPayloads(params: { records: Record; config?: OpenClawConfig; @@ -272,7 +292,7 @@ function createGuidedPostUpdatePluginOutcome(outcome: PluginUpdateOutcome): { outcome: PluginUpdateOutcome; warning?: PostUpdatePluginWarning; } { - if (outcome.status !== "error") { + if (outcome.status !== "error" && !isDisabledAfterFailureOutcome(outcome)) { return { outcome }; } const warning = createPostUpdatePluginWarning({ @@ -288,6 +308,10 @@ function createGuidedPostUpdatePluginOutcome(outcome: PluginUpdateOutcome): { }; } +function isDisabledAfterFailureOutcome(outcome: PluginUpdateOutcome): boolean { + return outcome.status === "skipped" && outcome.message.includes("after plugin update failure"); +} + export function shouldPrepareUpdatedInstallRestart(params: { updateMode: UpdateRunResult["mode"]; serviceInstalled: boolean; @@ -1077,6 +1101,7 @@ async function updatePluginsAfterCoreUpdate(params: { configSnapshot: Awaited>; opts: UpdateCommandOptions; timeoutMs: number; + pluginInstallRecords?: Record; }): Promise { if (!params.configSnapshot.valid) { if (!params.opts.json) { @@ -1116,7 +1141,8 @@ async function updatePluginsAfterCoreUpdate(params: { } const warnings: PostUpdatePluginWarning[] = []; - const pluginInstallRecords = await loadInstalledPluginIndexInstallRecords(); + const pluginInstallRecords = + params.pluginInstallRecords ?? (await loadInstalledPluginIndexInstallRecords()); const syncConfig = withPluginInstallRecords( params.configSnapshot.sourceConfig, pluginInstallRecords, @@ -1598,6 +1624,7 @@ async function runPostCorePluginUpdate(params: { configSnapshot: Awaited>; opts: UpdateCommandOptions; timeoutMs: number; + pluginInstallRecords?: Record; }): Promise { return await updatePluginsAfterCoreUpdate({ root: params.root, @@ -1605,6 +1632,7 @@ async function runPostCorePluginUpdate(params: { configSnapshot: params.configSnapshot, opts: params.opts, timeoutMs: params.timeoutMs, + pluginInstallRecords: params.pluginInstallRecords, }); } @@ -1706,6 +1734,27 @@ async function writePostCorePluginUpdateResultFile( await writeJson(filePath, result, { trailingNewline: true }); } +async function writePostCorePluginInstallRecordsFile( + filePath: string, + records: Record, +): Promise { + await fs.writeFile(filePath, `${JSON.stringify(records)}\n`, "utf-8"); +} + +async function readPostCorePluginInstallRecordsFile( + filePath: string | undefined, +): Promise | undefined> { + if (!filePath) { + return undefined; + } + try { + const parsed = JSON.parse(await fs.readFile(filePath, "utf-8")) as unknown; + return normalizePluginInstallRecordMap(parsed); + } catch { + return undefined; + } +} + async function readPostCorePluginUpdateResultFile( filePath: string, ): Promise { @@ -1751,6 +1800,7 @@ async function continuePostCoreUpdateInFreshProcess(params: { channel: "stable" | "beta" | "dev"; requestedChannel: "stable" | "beta" | "dev" | null; opts: UpdateCommandOptions; + pluginInstallRecords: Record; }): Promise<{ resumed: boolean; pluginUpdate?: PostCorePluginUpdateResult }> { const entryPath = await resolveGatewayInstallEntrypoint(params.root); if (!entryPath) { @@ -1772,8 +1822,10 @@ async function continuePostCoreUpdateInFreshProcess(params: { } const resultDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-update-post-core-")); const resultPath = path.join(resultDir, "plugins.json"); + const installRecordsPath = path.join(resultDir, "plugin-install-records.json"); try { + await writePostCorePluginInstallRecordsFile(installRecordsPath, params.pluginInstallRecords); const child = spawn(resolveNodeRunner(), argv, { stdio: "inherit", env: { @@ -1784,6 +1836,7 @@ async function continuePostCoreUpdateInFreshProcess(params: { ? { [POST_CORE_UPDATE_REQUESTED_CHANNEL_ENV]: params.requestedChannel } : {}), [POST_CORE_UPDATE_RESULT_PATH_ENV]: resultPath, + [POST_CORE_UPDATE_INSTALL_RECORDS_PATH_ENV]: installRecordsPath, }, }); @@ -1885,6 +1938,7 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise { const postCoreUpdateChannel = process.env[POST_CORE_UPDATE_CHANNEL_ENV]?.trim(); const postCoreRequestedChannelInput = process.env[POST_CORE_UPDATE_REQUESTED_CHANNEL_ENV]?.trim() ?? ""; + const postCoreInstallRecordsPath = process.env[POST_CORE_UPDATE_INSTALL_RECORDS_PATH_ENV]; const timeoutMs = parseTimeoutMsOrExit(opts.timeout); const shouldRestart = opts.restart !== false; @@ -1925,6 +1979,7 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise { configSnapshot: postCoreConfigSnapshot, opts, timeoutMs: updateStepTimeoutMs, + pluginInstallRecords: await readPostCorePluginInstallRecordsFile(postCoreInstallRecordsPath), }); if (process.env[POST_CORE_UPDATE_RESULT_PATH_ENV]) { await writePostCorePluginUpdateResultFile( @@ -2159,6 +2214,7 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise { const { progress, stop } = createUpdateProgress(showProgress); const startedAt = Date.now(); + const preUpdatePluginInstallRecords = await loadInstalledPluginIndexInstallRecords(); let prePackageServiceStop: PrePackageServiceStop | undefined; if (updateInstallKind === "package") { @@ -2319,6 +2375,7 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise { channel, requestedChannel, opts, + pluginInstallRecords: preUpdatePluginInstallRecords, }); pluginsUpdatedInFreshProcess = freshProcessResult.resumed; postCorePluginUpdate = freshProcessResult.pluginUpdate; @@ -2337,6 +2394,7 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise { configSnapshot: postUpdateConfigSnapshot, opts, timeoutMs: updateStepTimeoutMs, + pluginInstallRecords: preUpdatePluginInstallRecords, }); }