From f2e342b82e02029e006276d04f8462a13267e5e6 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 2 May 2026 17:34:29 -0700 Subject: [PATCH] fix(doctor): defer missing plugin payload repair during update --- .../missing-configured-plugin-install.test.ts | 172 ++++++++++++++++++ .../missing-configured-plugin-install.ts | 100 +++++++++- 2 files changed, 262 insertions(+), 10 deletions(-) diff --git a/src/commands/doctor/shared/missing-configured-plugin-install.test.ts b/src/commands/doctor/shared/missing-configured-plugin-install.test.ts index 69a256f622c..63259ecb94f 100644 --- a/src/commands/doctor/shared/missing-configured-plugin-install.test.ts +++ b/src/commands/doctor/shared/missing-configured-plugin-install.test.ts @@ -458,6 +458,178 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(result).toEqual({ changes: [], warnings: [] }); }); + it("does not download configured channel plugins that are still bundled", async () => { + mocks.listChannelPluginCatalogEntries.mockReturnValue([ + { + id: "matrix", + pluginId: "matrix", + origin: "bundled", + meta: { label: "Matrix" }, + install: { + npmSpec: "@openclaw/matrix", + }, + }, + ]); + mocks.loadPluginMetadataSnapshot.mockReturnValue({ + plugins: [ + { + id: "matrix", + origin: "bundled", + packageName: "@openclaw/matrix", + channels: ["matrix"], + }, + ], + diagnostics: [], + }); + + const { repairMissingConfiguredPluginInstalls } = + await import("./missing-configured-plugin-install.js"); + const result = await repairMissingConfiguredPluginInstalls({ + cfg: { + plugins: { + entries: { + matrix: { enabled: true }, + }, + }, + channels: { + matrix: { enabled: true, homeserver: "https://matrix.example.org" }, + }, + }, + env: {}, + }); + + expect(mocks.updateNpmInstalledPlugins).not.toHaveBeenCalled(); + expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); + expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); + expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); + expect(result).toEqual({ changes: [], warnings: [] }); + }); + + it("removes stale managed install records when the configured plugin is bundled", async () => { + const records = { + matrix: { + source: "npm", + spec: "@openclaw/matrix", + installPath: "/missing/matrix", + }, + }; + mocks.loadInstalledPluginIndexInstallRecords.mockResolvedValue(records); + mocks.listChannelPluginCatalogEntries.mockReturnValue([ + { + id: "matrix", + pluginId: "matrix", + origin: "bundled", + meta: { label: "Matrix" }, + install: { + npmSpec: "@openclaw/matrix", + }, + }, + ]); + mocks.loadPluginMetadataSnapshot.mockReturnValue({ + plugins: [ + { + id: "matrix", + origin: "bundled", + packageName: "@openclaw/matrix", + channels: ["matrix"], + }, + ], + diagnostics: [ + { + pluginId: "matrix", + message: "manifest without channelConfigs metadata", + }, + ], + }); + + const { repairMissingConfiguredPluginInstalls } = + await import("./missing-configured-plugin-install.js"); + const result = await repairMissingConfiguredPluginInstalls({ + cfg: { + plugins: { + entries: { + matrix: { enabled: true }, + }, + }, + channels: { + matrix: { enabled: true, homeserver: "https://matrix.example.org" }, + }, + }, + env: {}, + }); + + expect(mocks.updateNpmInstalledPlugins).not.toHaveBeenCalled(); + expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); + expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); + expect(mocks.writePersistedInstalledPluginIndexInstallRecords).toHaveBeenCalledWith( + {}, + { + env: {}, + }, + ); + expect(result).toEqual({ + changes: ['Removed stale managed install record for bundled plugin "matrix".'], + warnings: [], + }); + }); + + it("defers missing external payload repair during the package update doctor pass", async () => { + const records = { + discord: { + source: "npm", + spec: "@openclaw/discord", + installPath: "/missing/discord", + }, + }; + mocks.loadInstalledPluginIndexInstallRecords.mockResolvedValue(records); + mocks.listChannelPluginCatalogEntries.mockReturnValue([ + { + id: "discord", + pluginId: "discord", + meta: { label: "Discord" }, + install: { + npmSpec: "@openclaw/discord", + }, + }, + ]); + + const { repairMissingConfiguredPluginInstalls } = + await import("./missing-configured-plugin-install.js"); + const result = await repairMissingConfiguredPluginInstalls({ + cfg: { + plugins: { + entries: { + discord: { enabled: true }, + }, + }, + channels: { + discord: { enabled: true }, + }, + }, + env: { + OPENCLAW_UPDATE_IN_PROGRESS: "1", + }, + }); + + expect(mocks.updateNpmInstalledPlugins).not.toHaveBeenCalled(); + expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); + expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); + expect(mocks.writePersistedInstalledPluginIndexInstallRecords).toHaveBeenCalledWith( + {}, + { + env: { + OPENCLAW_UPDATE_IN_PROGRESS: "1", + }, + }, + ); + expect(result).toEqual({ + changes: [ + 'Deferred missing configured plugin "discord" install repair until post-update doctor.', + ], + warnings: [], + }); + }); + it("does not install configured plugins when plugins are globally disabled", async () => { mocks.listChannelPluginCatalogEntries.mockReturnValue([ { diff --git a/src/commands/doctor/shared/missing-configured-plugin-install.ts b/src/commands/doctor/shared/missing-configured-plugin-install.ts index bed83f0f49d..44bc2ab3e70 100644 --- a/src/commands/doctor/shared/missing-configured-plugin-install.ts +++ b/src/commands/doctor/shared/missing-configured-plugin-install.ts @@ -15,6 +15,7 @@ import { loadInstalledPluginIndexInstallRecords } from "../../../plugins/install import { writePersistedInstalledPluginIndexInstallRecords } from "../../../plugins/installed-plugin-index-records.js"; import { buildNpmResolutionInstallFields } from "../../../plugins/installs.js"; import { loadManifestMetadataSnapshot } from "../../../plugins/manifest-contract-eligibility.js"; +import type { PluginManifestRecord } from "../../../plugins/manifest-registry.js"; import type { PluginPackageInstall } from "../../../plugins/manifest.js"; import { listOfficialExternalPluginCatalogEntries, @@ -53,6 +54,7 @@ const RUNTIME_PLUGIN_INSTALL_CANDIDATES: readonly DownloadableInstallCandidate[] ]; const MISSING_CHANNEL_CONFIG_DESCRIPTOR_DIAGNOSTIC = "without channelConfigs metadata"; +const UPDATE_IN_PROGRESS_ENV = "OPENCLAW_UPDATE_IN_PROGRESS"; function shouldFallbackClawHubToNpm(result: { ok: false; code?: string }): boolean { return ( @@ -172,6 +174,9 @@ function collectDownloadableInstallCandidates(params: { env: params.env, excludeWorkspace: true, })) { + if (entry.origin === "bundled") { + continue; + } const pluginId = entry.pluginId ?? entry.id; if (params.blockedPluginIds?.has(pluginId)) { continue; @@ -305,6 +310,31 @@ function isInstalledRecordMissingOnDisk( return !existsSync(path.join(resolved, "package.json")); } +function isUpdatePackageDoctorPass(env: NodeJS.ProcessEnv): boolean { + return env[UPDATE_IN_PROGRESS_ENV] === "1"; +} + +function recordMatchesBundledPackage( + record: PluginInstallRecord, + bundled: PluginManifestRecord, +): boolean { + const packageName = bundled.packageName?.trim() || bundled.name?.trim(); + if (!packageName) { + return false; + } + if (record.source === "npm") { + return [record.spec, record.resolvedName, record.resolvedSpec].some((value) => + value?.trim().startsWith(packageName), + ); + } + if (record.source === "clawhub") { + return [record.clawhubPackage, record.spec].some((value) => + value?.trim().includes(packageName), + ); + } + return false; +} + async function installCandidate(params: { candidate: DownloadableInstallCandidate; records: Record; @@ -451,6 +481,11 @@ async function repairMissingPluginInstalls(params: { env, }); const knownIds = new Set(snapshot.plugins.map((plugin) => plugin.id)); + const bundledPluginsById = new Map( + snapshot.plugins + .filter((plugin) => plugin.origin === "bundled") + .map((plugin) => [plugin.id, plugin]), + ); const configuredPluginIdsWithStaleDescriptors = collectConfiguredPluginIdsWithMissingChannelConfigDescriptors({ snapshot, @@ -458,23 +493,60 @@ async function repairMissingPluginInstalls(params: { configuredChannelIds: params.channelIds, }); const records = await loadInstalledPluginIndexInstallRecords({ env }); - const missingRecordedPluginIds = Object.keys(records).filter( - (pluginId) => - (params.pluginIds.has(pluginId) && - (!knownIds.has(pluginId) || isInstalledRecordMissingOnDisk(records[pluginId], env))) || - configuredPluginIdsWithStaleDescriptors.has(pluginId), - ); const changes: string[] = []; const warnings: string[] = []; + const deferredPluginIds = new Set(); let nextRecords = records; + for (const [pluginId, record] of Object.entries(records)) { + const bundled = bundledPluginsById.get(pluginId); + if ( + !bundled || + !params.pluginIds.has(pluginId) || + !recordMatchesBundledPackage(record, bundled) + ) { + continue; + } + if (nextRecords === records) { + nextRecords = { ...records }; + } + delete nextRecords[pluginId]; + changes.push(`Removed stale managed install record for bundled plugin "${pluginId}".`); + } + + if (isUpdatePackageDoctorPass(env)) { + for (const pluginId of params.pluginIds) { + const record = nextRecords[pluginId]; + if (!record || !isInstalledRecordMissingOnDisk(record, env)) { + continue; + } + if (nextRecords === records) { + nextRecords = { ...records }; + } + delete nextRecords[pluginId]; + deferredPluginIds.add(pluginId); + changes.push( + `Deferred missing configured plugin "${pluginId}" install repair until post-update doctor.`, + ); + } + } + + const missingRecordedPluginIds = Object.keys(records).filter( + (pluginId) => + Object.hasOwn(nextRecords, pluginId) && + !bundledPluginsById.has(pluginId) && + ((params.pluginIds.has(pluginId) && + (!knownIds.has(pluginId) || isInstalledRecordMissingOnDisk(nextRecords[pluginId], env))) || + configuredPluginIdsWithStaleDescriptors.has(pluginId)), + ); + if (missingRecordedPluginIds.length > 0) { const updateResult = await updateNpmInstalledPlugins({ config: { ...params.cfg, plugins: { ...params.cfg.plugins, - installs: records, + installs: nextRecords, }, }, pluginIds: missingRecordedPluginIds, @@ -496,10 +568,15 @@ async function repairMissingPluginInstalls(params: { const missingPluginIds = new Set( [...params.pluginIds].filter((pluginId) => { + if (deferredPluginIds.has(pluginId)) { + return false; + } const hasRecord = Object.hasOwn(nextRecords, pluginId); return ( - (!knownIds.has(pluginId) && !hasRecord) || - (hasRecord && isInstalledRecordMissingOnDisk(nextRecords[pluginId], env)) + (!knownIds.has(pluginId) && !hasRecord && !bundledPluginsById.has(pluginId)) || + (hasRecord && + !bundledPluginsById.has(pluginId) && + isInstalledRecordMissingOnDisk(nextRecords[pluginId], env)) ); }), ); @@ -509,7 +586,10 @@ async function repairMissingPluginInstalls(params: { missingPluginIds, configuredPluginIds: params.pluginIds, configuredChannelIds: params.channelIds, - blockedPluginIds: params.blockedPluginIds, + blockedPluginIds: + deferredPluginIds.size > 0 + ? new Set([...(params.blockedPluginIds ?? []), ...deferredPluginIds]) + : params.blockedPluginIds, })) { const hasUsableRecord = Object.hasOwn(nextRecords, candidate.pluginId) &&