diff --git a/src/cli/plugins-install-command.ts b/src/cli/plugins-install-command.ts index 64b549bf4ea1..e36c9a39080f 100644 --- a/src/cli/plugins-install-command.ts +++ b/src/cli/plugins-install-command.ts @@ -540,6 +540,11 @@ async function loadConfigFromSnapshotForInstall( for (const mutation of await collectChannelDoctorStaleConfigMutations(snapshot.config, { env: process.env, })) { + if (mutation.warnings?.length) { + throw buildInvalidPluginInstallConfigError( + `${mutation.warnings.join("\n")} Run \`openclaw doctor --fix\` before reinstalling plugins.`, + ); + } nextConfig = mutation.config; } return { diff --git a/src/cli/plugins-install-config.test.ts b/src/cli/plugins-install-config.test.ts index 249d9d125725..78e38936237b 100644 --- a/src/cli/plugins-install-config.test.ts +++ b/src/cli/plugins-install-config.test.ts @@ -238,6 +238,33 @@ describe("loadConfigForInstall", () => { expect(result.config).toBe(snapshotCfg); }); + it("rejects plugin install recovery when stale channel cleanup reports warnings", async () => { + const snapshotCfg = { + plugins: { installs: { discord: { source: "path", installPath: "/gone" } } }, + } as unknown as OpenClawConfig; + readConfigFileSnapshotMock.mockResolvedValue( + makeSnapshot({ + parsed: { plugins: { installs: { discord: {} } } }, + config: snapshotCfg, + issues: [ + { path: "channels.discord", message: "unknown channel id: discord" }, + { path: "plugins.load.paths", message: "plugin: plugin path not found: /gone" }, + ], + }), + ); + collectChannelDoctorStaleConfigMutationsMock.mockResolvedValueOnce([ + { + changes: [], + config: snapshotCfg, + warnings: ["- channels.discord: channel stale cleanup failed."], + }, + ]); + + await expect(loadConfigForInstall(discordNpmRequest)).rejects.toThrow( + "channels.discord: channel stale cleanup failed", + ); + }); + it("rejects unrelated invalid config even during bundled-plugin reinstall recovery", async () => { readConfigFileSnapshotMock.mockResolvedValue( makeSnapshot({ diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts index 5be6684c699a..dd7a5ec9c172 100644 --- a/src/commands/doctor-config-flow.test.ts +++ b/src/commands/doctor-config-flow.test.ts @@ -1523,6 +1523,29 @@ describe("doctor config flow", () => { }); }); + it("emits warning-only channel stale cleanup diagnostics", async () => { + const channelDoctor = await import("./doctor/shared/channel-doctor.js"); + vi.mocked(channelDoctor.collectChannelDoctorStaleConfigMutations).mockResolvedValueOnce([ + { + changes: [], + config: { + channels: { + matrix: {}, + }, + }, + warnings: ["- channels.matrix: channel stale cleanup failed."], + }, + ]); + + const warnings = await collectDoctorWarnings({ + channels: { + matrix: {}, + }, + }); + + expect(warnings).toContain("- channels.matrix: channel stale cleanup failed."); + }); + it("reloads gateway secrets and refreshes auth status after auth profile repairs", async () => { runDoctorRepairSequenceMock.mockImplementation(async (params: { state: unknown }) => ({ state: params.state, diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 2a504f485977..35dd70a8119e 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -239,6 +239,10 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { candidate, { env: process.env }, )) { + emitDoctorNotes({ + note, + warningNotes: staleCleanup.warnings, + }); if (staleCleanup.changes.length === 0) { continue; } diff --git a/src/commands/doctor/shared/channel-doctor.test.ts b/src/commands/doctor/shared/channel-doctor.test.ts index 1d067f0ec98c..cc884e72dcc0 100644 --- a/src/commands/doctor/shared/channel-doctor.test.ts +++ b/src/commands/doctor/shared/channel-doctor.test.ts @@ -3,8 +3,11 @@ import { collectChannelDoctorCompatibilityMutations, collectChannelDoctorEmptyAllowlistExtraWarnings, collectChannelDoctorMutableAllowlistWarnings, + collectChannelDoctorPreviewWarnings, + collectChannelDoctorRepairMutations, collectChannelDoctorStaleConfigMutations, createChannelDoctorEmptyAllowlistPolicyHooks, + runChannelDoctorConfigSequences, } from "./channel-doctor.js"; const mocks = vi.hoisted(() => ({ @@ -386,4 +389,145 @@ describe("channel doctor compatibility mutations", () => { expect(collectEmptyAllowlistExtraWarnings).toHaveBeenCalledTimes(3); expect(shouldSkipDefaultEmptyGroupAllowlistWarning).toHaveBeenCalledTimes(1); }); + + it("reports throwing empty allowlist doctor extra-warning hooks", () => { + const collectEmptyAllowlistExtraWarnings = vi.fn(() => { + throw new Error("channel doctor extra hook exploded"); + }); + const shouldSkipDefaultEmptyGroupAllowlistWarning = vi.fn(() => { + throw new Error("channel doctor skip hook exploded"); + }); + const cfg = createMatrixEnabledConfig(); + mockReadOnlyMatrixPlugin({ + collectEmptyAllowlistExtraWarnings, + shouldSkipDefaultEmptyGroupAllowlistWarning, + }); + + const hooks = createChannelDoctorEmptyAllowlistPolicyHooks({ cfg: cfg as never }); + + expect( + hooks.extraWarningsForAccount({ + account: {}, + channelName: "matrix", + prefix: "channels.matrix", + }), + ).toEqual([ + "- channels.matrix: channel plugin doctor hook collectEmptyAllowlistExtraWarnings failed (channel doctor extra hook exploded). Fix or disable this channel plugin before relying on channel doctor diagnostics.", + ]); + expect( + hooks.shouldSkipDefaultEmptyGroupAllowlistWarning({ + account: {}, + channelName: "matrix", + prefix: "channels.matrix", + }), + ).toBe(false); + }); + + it("keeps healthy preview warnings when another channel doctor hook throws", async () => { + const collectPreviewWarnings = vi.fn(() => { + throw new Error("channel doctor preview hook exploded"); + }); + mocks.resolveReadOnlyChannelPluginsForConfig.mockReturnValue({ + plugins: [ + { + id: "matrix", + doctor: { + collectPreviewWarnings, + }, + }, + { + id: "slack", + doctor: { + collectPreviewWarnings: () => ["slack preview warning"], + }, + }, + ], + }); + + const warnings = await collectChannelDoctorPreviewWarnings({ + cfg: { + channels: { + matrix: {}, + slack: {}, + }, + } as never, + doctorFixCommand: "openclaw doctor --fix", + }); + + expect(warnings).toEqual([ + "- channels.matrix: channel plugin doctor hook collectPreviewWarnings failed (channel doctor preview hook exploded). Fix or disable this channel plugin before relying on channel doctor diagnostics.", + "slack preview warning", + ]); + }); + + it("reports throwing channel doctor repair and mutation hooks", async () => { + const cfg = createMatrixEnabledConfig(); + mockReadOnlyMatrixPlugin({ + cleanStaleConfig: () => { + throw new Error("channel doctor stale hook exploded"); + }, + collectMutableAllowlistWarnings: () => { + throw new Error("channel doctor mutable hook exploded"); + }, + normalizeCompatibilityConfig: () => { + throw new Error("channel doctor compat hook exploded"); + }, + repairConfig: () => { + throw new Error("channel doctor repair hook exploded"); + }, + runConfigSequence: () => { + throw new Error("channel doctor sequence hook exploded"); + }, + }); + + expect(collectChannelDoctorCompatibilityMutations(cfg as never)).toEqual([ + { + changes: [], + config: cfg, + warnings: [ + "- channels.matrix: channel plugin doctor hook normalizeCompatibilityConfig failed (channel doctor compat hook exploded). Fix or disable this channel plugin before relying on channel doctor diagnostics.", + ], + }, + ]); + await expect(collectChannelDoctorStaleConfigMutations(cfg as never)).resolves.toEqual([ + { + changes: [], + config: cfg, + warnings: [ + "- channels.matrix: channel plugin doctor hook cleanStaleConfig failed (channel doctor stale hook exploded). Fix or disable this channel plugin before relying on channel doctor diagnostics.", + ], + }, + ]); + await expect( + collectChannelDoctorMutableAllowlistWarnings({ cfg: cfg as never }), + ).resolves.toEqual([ + "- channels.matrix: channel plugin doctor hook collectMutableAllowlistWarnings failed (channel doctor mutable hook exploded). Fix or disable this channel plugin before relying on channel doctor diagnostics.", + ]); + await expect( + runChannelDoctorConfigSequences({ + cfg: cfg as never, + env: {}, + shouldRepair: false, + }), + ).resolves.toEqual({ + changeNotes: [], + warningNotes: [ + "- channels.matrix: channel plugin doctor hook runConfigSequence failed (channel doctor sequence hook exploded). Fix or disable this channel plugin before relying on channel doctor diagnostics.", + ], + }); + await expect( + collectChannelDoctorRepairMutations({ + cfg: cfg as never, + doctorFixCommand: "openclaw doctor --fix", + }), + ).resolves.toEqual([ + { + changes: [], + config: cfg, + warnings: [ + "- channels.matrix: channel plugin doctor hook repairConfig failed (channel doctor repair hook exploded). Fix or disable this channel plugin before relying on channel doctor diagnostics.", + ], + }, + ]); + }); }); diff --git a/src/commands/doctor/shared/channel-doctor.ts b/src/commands/doctor/shared/channel-doctor.ts index ab1d3642c4f9..693cfb13be65 100644 --- a/src/commands/doctor/shared/channel-doctor.ts +++ b/src/commands/doctor/shared/channel-doctor.ts @@ -1,4 +1,5 @@ import { normalizeOptionalLowercaseString } from "@openclaw/normalization-core/string-coerce"; +import { sanitizeForLog } from "../../../../packages/terminal-core/src/ansi.js"; import { getBundledChannelPlugin, getBundledChannelSetupPlugin, @@ -12,8 +13,10 @@ import type { ChannelDoctorSequenceResult, } from "../../../channels/plugins/types.adapters.js"; import type { OpenClawConfig } from "../../../config/types.openclaw.js"; +import { formatErrorMessage } from "../../../infra/errors.js"; type ChannelDoctorEntry = { + id: string; doctor: ChannelDoctorAdapter; }; @@ -223,11 +226,21 @@ function listChannelDoctorEntries( if (!doctor) { continue; } - entries.push({ doctor }); + entries.push({ doctor, id }); } return entries; } +function formatChannelDoctorHookFailure(params: { + channelId: string; + error: unknown; + hookName: keyof ChannelDoctorAdapter; +}): string { + return sanitizeForLog( + `- channels.${params.channelId}: channel plugin doctor hook ${params.hookName} failed (${formatErrorMessage(params.error)}). Fix or disable this channel plugin before relying on channel doctor diagnostics.`, + ); +} + function toPluginEmptyAllowlistContext({ cfg: _cfg, ...params @@ -242,7 +255,19 @@ function collectEmptyAllowlistExtraWarningsForEntries( const warnings: string[] = []; const pluginParams = toPluginEmptyAllowlistContext(params); for (const entry of entries) { - const lines = entry.doctor.collectEmptyAllowlistExtraWarnings?.(pluginParams); + let lines: string[] | undefined; + try { + lines = entry.doctor.collectEmptyAllowlistExtraWarnings?.(pluginParams); + } catch (error) { + warnings.push( + formatChannelDoctorHookFailure({ + channelId: entry.id, + error, + hookName: "collectEmptyAllowlistExtraWarnings", + }), + ); + continue; + } if (lines?.length) { warnings.push(...lines); } @@ -255,9 +280,16 @@ function shouldSkipDefaultEmptyGroupAllowlistWarningForEntries( params: ChannelDoctorEmptyAllowlistLookupParams, ): boolean { const pluginParams = toPluginEmptyAllowlistContext(params); - return entries.some( - (entry) => entry.doctor.shouldSkipDefaultEmptyGroupAllowlistWarning?.(pluginParams) === true, - ); + for (const entry of entries) { + try { + if (entry.doctor.shouldSkipDefaultEmptyGroupAllowlistWarning?.(pluginParams) === true) { + return true; + } + } catch { + // A bad plugin doctor hook must not suppress the core empty-allowlist warning. + } + } + return false; } export function createChannelDoctorEmptyAllowlistPolicyHooks( @@ -296,7 +328,19 @@ export async function runChannelDoctorConfigSequences(params: { cfg: params.cfg, env: params.env, })) { - const result = await entry.doctor.runConfigSequence?.(params); + let result: ChannelDoctorSequenceResult | undefined; + try { + result = await entry.doctor.runConfigSequence?.(params); + } catch (error) { + warningNotes.push( + formatChannelDoctorHookFailure({ + channelId: entry.id, + error, + hookName: "runConfigSequence", + }), + ); + continue; + } if (!result) { continue; } @@ -317,7 +361,23 @@ export function collectChannelDoctorCompatibilityMutations( const mutations: ChannelDoctorConfigMutation[] = []; let nextCfg = cfg; for (const entry of listChannelDoctorEntries(channelIds, { cfg, env: options.env })) { - const mutation = entry.doctor.normalizeCompatibilityConfig?.({ cfg: nextCfg }); + let mutation: ChannelDoctorConfigMutation | undefined; + try { + mutation = entry.doctor.normalizeCompatibilityConfig?.({ cfg: nextCfg }); + } catch (error) { + mutations.push({ + config: nextCfg, + changes: [], + warnings: [ + formatChannelDoctorHookFailure({ + channelId: entry.id, + error, + hookName: "normalizeCompatibilityConfig", + }), + ], + }); + continue; + } if (!mutation || mutation.changes.length === 0) { continue; } @@ -337,7 +397,23 @@ export async function collectChannelDoctorStaleConfigMutations( cfg, env: options.env, })) { - const mutation = await entry.doctor.cleanStaleConfig?.({ cfg: nextCfg }); + let mutation: ChannelDoctorConfigMutation | undefined; + try { + mutation = await entry.doctor.cleanStaleConfig?.({ cfg: nextCfg }); + } catch (error) { + mutations.push({ + config: nextCfg, + changes: [], + warnings: [ + formatChannelDoctorHookFailure({ + channelId: entry.id, + error, + hookName: "cleanStaleConfig", + }), + ], + }); + continue; + } if (!mutation || mutation.changes.length === 0) { continue; } @@ -357,7 +433,19 @@ export async function collectChannelDoctorPreviewWarnings(params: { cfg: params.cfg, env: params.env, })) { - const lines = await entry.doctor.collectPreviewWarnings?.(params); + let lines: string[] | undefined; + try { + lines = await entry.doctor.collectPreviewWarnings?.(params); + } catch (error) { + warnings.push( + formatChannelDoctorHookFailure({ + channelId: entry.id, + error, + hookName: "collectPreviewWarnings", + }), + ); + continue; + } if (lines?.length) { warnings.push(...lines); } @@ -374,7 +462,19 @@ export async function collectChannelDoctorMutableAllowlistWarnings(params: { cfg: params.cfg, env: params.env, })) { - const lines = await entry.doctor.collectMutableAllowlistWarnings?.(params); + let lines: string[] | undefined; + try { + lines = await entry.doctor.collectMutableAllowlistWarnings?.(params); + } catch (error) { + warnings.push( + formatChannelDoctorHookFailure({ + channelId: entry.id, + error, + hookName: "collectMutableAllowlistWarnings", + }), + ); + continue; + } if (lines?.length) { warnings.push(...lines); } @@ -393,10 +493,26 @@ export async function collectChannelDoctorRepairMutations(params: { cfg: params.cfg, env: params.env, })) { - const mutation = await entry.doctor.repairConfig?.({ - cfg: nextCfg, - doctorFixCommand: params.doctorFixCommand, - }); + let mutation: ChannelDoctorConfigMutation | undefined; + try { + mutation = await entry.doctor.repairConfig?.({ + cfg: nextCfg, + doctorFixCommand: params.doctorFixCommand, + }); + } catch (error) { + mutations.push({ + config: nextCfg, + changes: [], + warnings: [ + formatChannelDoctorHookFailure({ + channelId: entry.id, + error, + hookName: "repairConfig", + }), + ], + }); + continue; + } if (!mutation || mutation.changes.length === 0) { if (mutation?.warnings?.length) { mutations.push({ config: nextCfg, changes: [], warnings: mutation.warnings });