diff --git a/src/commands/doctor/emit-notes.test.ts b/src/commands/doctor/emit-notes.test.ts index 0f2211a8525..26e6f531f67 100644 --- a/src/commands/doctor/emit-notes.test.ts +++ b/src/commands/doctor/emit-notes.test.ts @@ -34,12 +34,12 @@ describe("doctor note emission", () => { emitDoctorNotes({ note, - changeNotes: ["change \u001B[31mred\u001B[0m"], + changeNotes: ["change \u001B[31mred\u001B[0m\nnext line"], warningNotes: ["warning \u001B]8;;https://example.test\u001B\\link\u001B]8;;\u001B\\\r"], }); expect(note.mock.calls).toEqual([ - ["change red", "Doctor changes"], + ["change red\nnext line", "Doctor changes"], ["warning link", "Doctor warnings"], ]); }); diff --git a/src/commands/doctor/emit-notes.ts b/src/commands/doctor/emit-notes.ts index 2cc7e458692..c2e4e20e1ea 100644 --- a/src/commands/doctor/emit-notes.ts +++ b/src/commands/doctor/emit-notes.ts @@ -1,14 +1,21 @@ import { sanitizeForLog } from "../../terminal/ansi.js"; +function sanitizeDoctorNote(note: string): string { + return note + .split("\n") + .map((line) => sanitizeForLog(line)) + .join("\n"); +} + export function emitDoctorNotes(params: { note: (message: string, title?: string) => void; changeNotes?: string[]; warningNotes?: string[]; }): void { for (const change of params.changeNotes ?? []) { - params.note(sanitizeForLog(change), "Doctor changes"); + params.note(sanitizeDoctorNote(change), "Doctor changes"); } for (const warning of params.warningNotes ?? []) { - params.note(sanitizeForLog(warning), "Doctor warnings"); + params.note(sanitizeDoctorNote(warning), "Doctor warnings"); } } diff --git a/src/commands/doctor/repair-sequencing.test.ts b/src/commands/doctor/repair-sequencing.test.ts index 20cc6eeafcb..ac77ea50c55 100644 --- a/src/commands/doctor/repair-sequencing.test.ts +++ b/src/commands/doctor/repair-sequencing.test.ts @@ -35,7 +35,10 @@ vi.mock("./shared/channel-doctor.js", () => ({ } return []; }, - collectChannelDoctorEmptyAllowlistExtraWarnings: () => [], + createChannelDoctorEmptyAllowlistPolicyHooks: () => ({ + extraWarningsForAccount: () => [], + shouldSkipDefaultEmptyGroupAllowlistWarning: () => false, + }), })); vi.mock("./shared/empty-allowlist-scan.js", () => ({ diff --git a/src/commands/doctor/repair-sequencing.ts b/src/commands/doctor/repair-sequencing.ts index dc6aa722bf3..abb87d5bc1d 100644 --- a/src/commands/doctor/repair-sequencing.ts +++ b/src/commands/doctor/repair-sequencing.ts @@ -2,7 +2,7 @@ import { sanitizeForLog } from "../../terminal/ansi.js"; import { maybeRepairAllowlistPolicyAllowFrom } from "./shared/allowlist-policy-repair.js"; import { maybeRepairBundledPluginLoadPaths } from "./shared/bundled-plugin-load-paths.js"; import { - collectChannelDoctorEmptyAllowlistExtraWarnings, + createChannelDoctorEmptyAllowlistPolicyHooks, collectChannelDoctorRepairMutations, } from "./shared/channel-doctor.js"; import { @@ -63,7 +63,7 @@ export async function runDoctorRepairSequence(params: { const emptyAllowlistWarnings = scanEmptyAllowlistPolicyWarnings(state.candidate, { doctorFixCommand: params.doctorFixCommand, env, - extraWarningsForAccount: collectChannelDoctorEmptyAllowlistExtraWarnings, + ...createChannelDoctorEmptyAllowlistPolicyHooks({ cfg: state.candidate, env }), }); if (emptyAllowlistWarnings.length > 0) { warningNotes.push(sanitizeLines(emptyAllowlistWarnings)); diff --git a/src/commands/doctor/shared/channel-doctor.test.ts b/src/commands/doctor/shared/channel-doctor.test.ts index 49f9dd76e34..1f978168cef 100644 --- a/src/commands/doctor/shared/channel-doctor.test.ts +++ b/src/commands/doctor/shared/channel-doctor.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { collectChannelDoctorCompatibilityMutations, collectChannelDoctorEmptyAllowlistExtraWarnings, + createChannelDoctorEmptyAllowlistPolicyHooks, } from "./channel-doctor.js"; const mocks = vi.hoisted(() => ({ @@ -247,4 +248,71 @@ describe("channel doctor compatibility mutations", () => { expect.objectContaining({ cfg }), ); }); + + it("reuses empty allowlist doctor entries across per-account hooks", () => { + const collectEmptyAllowlistExtraWarnings = vi.fn(({ prefix }: { prefix: string }) => [ + `${prefix} extra`, + ]); + const shouldSkipDefaultEmptyGroupAllowlistWarning = vi.fn(() => true); + const cfg = { + channels: { + matrix: { + accounts: { + work: {}, + personal: {}, + }, + }, + }, + }; + const env = { OPENCLAW_HOME: "/tmp/openclaw-test-home" }; + mocks.resolveReadOnlyChannelPluginsForConfig.mockReturnValue({ + plugins: [ + { + id: "matrix", + doctor: { + collectEmptyAllowlistExtraWarnings, + shouldSkipDefaultEmptyGroupAllowlistWarning, + }, + }, + ], + }); + + const hooks = createChannelDoctorEmptyAllowlistPolicyHooks({ cfg: cfg as never, env }); + + expect( + hooks.extraWarningsForAccount({ + account: {}, + channelName: "matrix", + cfg: cfg as never, + env, + prefix: "channels.matrix.accounts.work", + }), + ).toEqual(["channels.matrix.accounts.work extra"]); + expect( + hooks.shouldSkipDefaultEmptyGroupAllowlistWarning({ + account: {}, + channelName: "matrix", + cfg: cfg as never, + env, + prefix: "channels.matrix.accounts.work", + }), + ).toBe(true); + expect( + hooks.extraWarningsForAccount({ + account: {}, + channelName: "matrix", + cfg: cfg as never, + env, + prefix: "channels.matrix.accounts.personal", + }), + ).toEqual(["channels.matrix.accounts.personal extra"]); + + expect(mocks.resolveReadOnlyChannelPluginsForConfig).toHaveBeenCalledTimes(1); + expect(mocks.resolveReadOnlyChannelPluginsForConfig).toHaveBeenCalledWith(cfg, { + env, + includePersistedAuthState: false, + }); + expect(collectEmptyAllowlistExtraWarnings).toHaveBeenCalledTimes(2); + expect(shouldSkipDefaultEmptyGroupAllowlistWarning).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/commands/doctor/shared/channel-doctor.ts b/src/commands/doctor/shared/channel-doctor.ts index 8f435227fbb..3ec365ac014 100644 --- a/src/commands/doctor/shared/channel-doctor.ts +++ b/src/commands/doctor/shared/channel-doctor.ts @@ -21,6 +21,13 @@ type ChannelDoctorLookupContext = { env?: NodeJS.ProcessEnv; }; +export type ChannelDoctorEmptyAllowlistPolicyHooks = { + extraWarningsForAccount: (params: ChannelDoctorEmptyAllowlistAccountContext) => string[]; + shouldSkipDefaultEmptyGroupAllowlistWarning: ( + params: ChannelDoctorEmptyAllowlistAccountContext, + ) => boolean; +}; + function collectConfiguredChannelIds(cfg: OpenClawConfig): string[] { const channels = cfg.channels && typeof cfg.channels === "object" && !Array.isArray(cfg.channels) @@ -111,6 +118,53 @@ function listChannelDoctorEntries( return [...byId.values()]; } +function collectEmptyAllowlistExtraWarningsForEntries( + entries: readonly ChannelDoctorEntry[], + params: ChannelDoctorEmptyAllowlistAccountContext, +): string[] { + const warnings: string[] = []; + for (const entry of entries) { + const lines = entry.doctor.collectEmptyAllowlistExtraWarnings?.(params); + if (lines?.length) { + warnings.push(...lines); + } + } + return warnings; +} + +function shouldSkipDefaultEmptyGroupAllowlistWarningForEntries( + entries: readonly ChannelDoctorEntry[], + params: ChannelDoctorEmptyAllowlistAccountContext, +): boolean { + return entries.some( + (entry) => entry.doctor.shouldSkipDefaultEmptyGroupAllowlistWarning?.(params) === true, + ); +} + +export function createChannelDoctorEmptyAllowlistPolicyHooks( + context: ChannelDoctorLookupContext, +): ChannelDoctorEmptyAllowlistPolicyHooks { + const entriesByChannel = new Map(); + const entriesForChannel = (channelName: string) => { + const existing = entriesByChannel.get(channelName); + if (existing) { + return existing; + } + const entries = listChannelDoctorEntries([channelName], context); + entriesByChannel.set(channelName, entries); + return entries; + }; + return { + extraWarningsForAccount: (params) => + collectEmptyAllowlistExtraWarningsForEntries(entriesForChannel(params.channelName), params), + shouldSkipDefaultEmptyGroupAllowlistWarning: (params) => + shouldSkipDefaultEmptyGroupAllowlistWarningForEntries( + entriesForChannel(params.channelName), + params, + ), + }; +} + export async function runChannelDoctorConfigSequences(params: { cfg: OpenClawConfig; env: NodeJS.ProcessEnv; @@ -238,24 +292,23 @@ export async function collectChannelDoctorRepairMutations(params: { export function collectChannelDoctorEmptyAllowlistExtraWarnings( params: ChannelDoctorEmptyAllowlistAccountContext, ): string[] { - const warnings: string[] = []; - for (const entry of listChannelDoctorEntries([params.channelName], { - cfg: params.cfg ?? {}, - env: params.env, - })) { - const lines = entry.doctor.collectEmptyAllowlistExtraWarnings?.(params); - if (lines?.length) { - warnings.push(...lines); - } - } - return warnings; + return collectEmptyAllowlistExtraWarningsForEntries( + listChannelDoctorEntries([params.channelName], { + cfg: params.cfg ?? {}, + env: params.env, + }), + params, + ); } export function shouldSkipChannelDoctorDefaultEmptyGroupAllowlistWarning( params: ChannelDoctorEmptyAllowlistAccountContext, ): boolean { - return listChannelDoctorEntries([params.channelName], { - cfg: params.cfg ?? {}, - env: params.env, - }).some((entry) => entry.doctor.shouldSkipDefaultEmptyGroupAllowlistWarning?.(params) === true); + return shouldSkipDefaultEmptyGroupAllowlistWarningForEntries( + listChannelDoctorEntries([params.channelName], { + cfg: params.cfg ?? {}, + env: params.env, + }), + params, + ); } diff --git a/src/commands/doctor/shared/empty-allowlist-policy.ts b/src/commands/doctor/shared/empty-allowlist-policy.ts index be60b06c79a..0a52a6e5ee4 100644 --- a/src/commands/doctor/shared/empty-allowlist-policy.ts +++ b/src/commands/doctor/shared/empty-allowlist-policy.ts @@ -12,6 +12,7 @@ type CollectEmptyAllowlistPolicyWarningsParams = { env?: NodeJS.ProcessEnv; parent?: DoctorAccountRecord; prefix: string; + shouldSkipDefaultEmptyGroupAllowlistWarning?: typeof shouldSkipChannelDoctorDefaultEmptyGroupAllowlistWarning; }; function usesSenderBasedGroupAllowlist(channelName?: string): boolean { @@ -67,7 +68,10 @@ export function collectEmptyAllowlistPolicyWarningsForAccount( if ( params.channelName && - shouldSkipChannelDoctorDefaultEmptyGroupAllowlistWarning({ + ( + params.shouldSkipDefaultEmptyGroupAllowlistWarning ?? + shouldSkipChannelDoctorDefaultEmptyGroupAllowlistWarning + )({ account: params.account, channelName: params.channelName, cfg: params.cfg, diff --git a/src/commands/doctor/shared/empty-allowlist-scan.ts b/src/commands/doctor/shared/empty-allowlist-scan.ts index 53c0e70b0e3..2afd15a9e74 100644 --- a/src/commands/doctor/shared/empty-allowlist-scan.ts +++ b/src/commands/doctor/shared/empty-allowlist-scan.ts @@ -1,3 +1,4 @@ +import type { ChannelDoctorEmptyAllowlistAccountContext } from "../../../channels/plugins/types.adapters.js"; import type { OpenClawConfig } from "../../../config/types.openclaw.js"; import type { DoctorAccountRecord, DoctorAllowFromList } from "../types.js"; import { collectEmptyAllowlistPolicyWarningsForAccount } from "./empty-allowlist-policy.js"; @@ -17,7 +18,10 @@ export type EmptyAllowlistAccountScanParams = { type ScanEmptyAllowlistPolicyWarningsParams = { doctorFixCommand: string; env?: NodeJS.ProcessEnv; - extraWarningsForAccount?: (params: EmptyAllowlistAccountScanParams) => string[]; + extraWarningsForAccount?: (params: ChannelDoctorEmptyAllowlistAccountContext) => string[]; + shouldSkipDefaultEmptyGroupAllowlistWarning?: ( + params: ChannelDoctorEmptyAllowlistAccountContext, + ) => boolean; }; export function scanEmptyAllowlistPolicyWarnings( @@ -61,6 +65,8 @@ export function scanEmptyAllowlistPolicyWarnings( env: params.env, parent, prefix, + shouldSkipDefaultEmptyGroupAllowlistWarning: + params.shouldSkipDefaultEmptyGroupAllowlistWarning, }), ); if (params.extraWarningsForAccount) { diff --git a/src/commands/doctor/shared/preview-warnings.test.ts b/src/commands/doctor/shared/preview-warnings.test.ts index 03be7a4bd0f..9bf5ba88b1a 100644 --- a/src/commands/doctor/shared/preview-warnings.test.ts +++ b/src/commands/doctor/shared/preview-warnings.test.ts @@ -50,6 +50,10 @@ vi.mock("./channel-doctor.js", () => ({ ]; }, ), + createChannelDoctorEmptyAllowlistPolicyHooks: vi.fn(() => ({ + extraWarningsForAccount: () => [], + shouldSkipDefaultEmptyGroupAllowlistWarning: () => false, + })), shouldSkipChannelDoctorDefaultEmptyGroupAllowlistWarning: vi.fn(() => false), })); diff --git a/src/commands/doctor/shared/preview-warnings.ts b/src/commands/doctor/shared/preview-warnings.ts index e8d82381392..c0de5830623 100644 --- a/src/commands/doctor/shared/preview-warnings.ts +++ b/src/commands/doctor/shared/preview-warnings.ts @@ -156,12 +156,18 @@ export async function collectDoctorPreviewWarnings(params: { } if (hasChannelConfig) { - const { collectChannelDoctorEmptyAllowlistExtraWarnings } = await loadChannelDoctorModule(); + const { createChannelDoctorEmptyAllowlistPolicyHooks } = await loadChannelDoctorModule(); const { scanEmptyAllowlistPolicyWarnings } = await import("./empty-allowlist-scan.js"); + const emptyAllowlistHooks = createChannelDoctorEmptyAllowlistPolicyHooks({ + cfg: params.cfg, + env, + }); const emptyAllowlistWarnings = scanEmptyAllowlistPolicyWarnings(params.cfg, { doctorFixCommand: params.doctorFixCommand, env, - extraWarningsForAccount: collectChannelDoctorEmptyAllowlistExtraWarnings, + extraWarningsForAccount: emptyAllowlistHooks.extraWarningsForAccount, + shouldSkipDefaultEmptyGroupAllowlistWarning: + emptyAllowlistHooks.shouldSkipDefaultEmptyGroupAllowlistWarning, }).filter( (warning) => !(