fix(doctor): isolate channel doctor hook failures

This commit is contained in:
Vincent Koc
2026-06-04 05:58:25 +02:00
parent 32282418eb
commit 001ef70df6
6 changed files with 333 additions and 14 deletions

View File

@@ -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 {

View File

@@ -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({

View File

@@ -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,

View File

@@ -239,6 +239,10 @@ export async function loadAndMaybeMigrateDoctorConfig(params: {
candidate,
{ env: process.env },
)) {
emitDoctorNotes({
note,
warningNotes: staleCleanup.warnings,
});
if (staleCleanup.changes.length === 0) {
continue;
}

View File

@@ -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.",
],
},
]);
});
});

View File

@@ -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 });