diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fbd6cd8064..0f777261256 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Docs: https://docs.openclaw.ai - Control UI/config: preserve intentionally empty raw config snapshots when clearing pending updates so reset restores the original bytes instead of synthesizing JSON for blank config files. (#68178) Thanks @BunsDev. - memory-core/dreaming: surface a `Dreaming status: blocked` line in `openclaw memory status` when dreaming is enabled but the heartbeat that drives the managed cron is not firing for the default agent, and add a Troubleshooting section to the dreaming docs covering the two common causes (per-agent `heartbeat` blocks excluding `main`, and `heartbeat.every` set to `0`/empty/invalid), so the silent failure described in #69843 becomes legible on the status surface. - Cron/run-log: report generic `message` tool sends under the resolved delivery channel when they match the cron target, while preserving account-specific mismatch checks for delivery traces. (#69940) Thanks @davehappyminion. +- Doctor/channels: merge configured-channel doctor hooks across read-only, loaded, setup, and runtime plugin discovery so partial adapters no longer hide runtime-only compatibility repair or allowlist warnings, preserve disabled-channel opt-outs, and ignore malformed hook values before they can mask valid fallbacks. (#69919) Thanks @gumadeiras. ## 2026.4.21 diff --git a/src/cli/plugins-cli-test-helpers.ts b/src/cli/plugins-cli-test-helpers.ts index 70990755210..4b3f24b0744 100644 --- a/src/cli/plugins-cli-test-helpers.ts +++ b/src/cli/plugins-cli-test-helpers.ts @@ -188,18 +188,22 @@ vi.mock("../plugins/status.js", () => ({ )) as (typeof import("../plugins/status.js"))["buildPluginCompatibilityNotices"], })); -vi.mock("../plugins/slots.js", () => ({ - applyExclusiveSlotSelection: (( - params: Parameters<(typeof import("../plugins/slots.js"))["applyExclusiveSlotSelection"]>[0], - ) => - invokeMock< - [Parameters<(typeof import("../plugins/slots.js"))["applyExclusiveSlotSelection"]>[0]], - ReturnType<(typeof import("../plugins/slots.js"))["applyExclusiveSlotSelection"]> - >( - applyExclusiveSlotSelection, - params, - )) as (typeof import("../plugins/slots.js"))["applyExclusiveSlotSelection"], -})); +vi.mock("../plugins/slots.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + applyExclusiveSlotSelection: (( + params: Parameters<(typeof import("../plugins/slots.js"))["applyExclusiveSlotSelection"]>[0], + ) => + invokeMock< + [Parameters<(typeof import("../plugins/slots.js"))["applyExclusiveSlotSelection"]>[0]], + ReturnType<(typeof import("../plugins/slots.js"))["applyExclusiveSlotSelection"]> + >( + applyExclusiveSlotSelection, + params, + )) as (typeof import("../plugins/slots.js"))["applyExclusiveSlotSelection"], + }; +}); vi.mock("../plugins/uninstall.js", () => ({ uninstallPlugin: (( diff --git a/src/cli/plugins-install-command.ts b/src/cli/plugins-install-command.ts index adce3241291..d587fc1e556 100644 --- a/src/cli/plugins-install-command.ts +++ b/src/cli/plugins-install-command.ts @@ -255,7 +255,9 @@ async function loadConfigFromSnapshotForInstall( ); } let nextConfig = snapshot.config; - for (const mutation of await collectChannelDoctorStaleConfigMutations(snapshot.config)) { + for (const mutation of await collectChannelDoctorStaleConfigMutations(snapshot.config, { + env: process.env, + })) { nextConfig = mutation.config; } return nextConfig; diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts index 2c9a6d97d0b..aea2acb9e0a 100644 --- a/src/commands/doctor-config-flow.test.ts +++ b/src/commands/doctor-config-flow.test.ts @@ -935,26 +935,26 @@ vi.mock("./doctor/shared/channel-doctor.js", () => { return !groups && !hasOwnStringArray(groupAllowFrom); } + function collectTelegramFirstTimeExtraWarnings(params: { + account: Record; + channelName: string; + parent?: Record; + prefix: string; + }): string[] { + if ( + params.channelName !== "telegram" || + !isTelegramFirstTimeAccount({ account: params.account, parent: params.parent }) + ) { + return []; + } + return [ + `- ${params.prefix}: Telegram is in first-time setup mode. DMs use pairing mode. Group messages stay blocked until you add allowed chats under ${params.prefix}.groups (and optional sender IDs under ${params.prefix}.groupAllowFrom), or set ${params.prefix}.groupPolicy to "open" if you want broad group access.`, + ]; + } + return { collectChannelDoctorCompatibilityMutations: vi.fn(collectCompatibilityMutations), - collectChannelDoctorEmptyAllowlistExtraWarnings: vi.fn( - (params: { - account: Record; - channelName: string; - parent?: Record; - prefix: string; - }) => { - if ( - params.channelName !== "telegram" || - !isTelegramFirstTimeAccount({ account: params.account, parent: params.parent }) - ) { - return []; - } - return [ - `- ${params.prefix}: Telegram is in first-time setup mode. DMs use pairing mode. Group messages stay blocked until you add allowed chats under ${params.prefix}.groups (and optional sender IDs under ${params.prefix}.groupAllowFrom), or set ${params.prefix}.groupPolicy to "open" if you want broad group access.`, - ]; - }, - ), + collectChannelDoctorEmptyAllowlistExtraWarnings: vi.fn(collectTelegramFirstTimeExtraWarnings), collectChannelDoctorMutableAllowlistWarnings: vi.fn( ({ cfg }: { cfg: { channels?: Record } }) => { const zalouser = asRecord(cfg.channels?.zalouser); @@ -997,6 +997,11 @@ vi.mock("./doctor/shared/channel-doctor.js", () => { }, ), collectChannelDoctorStaleConfigMutations: vi.fn(async () => []), + createChannelDoctorEmptyAllowlistPolicyHooks: vi.fn(() => ({ + extraWarningsForAccount: collectTelegramFirstTimeExtraWarnings, + shouldSkipDefaultEmptyGroupAllowlistWarning: ({ channelName }: { channelName: string }) => + channelName === "googlechat" || channelName === "telegram", + })), runChannelDoctorConfigSequences: vi.fn(async () => ({ changeNotes: [], warningNotes: [] })), shouldSkipChannelDoctorDefaultEmptyGroupAllowlistWarning: vi.fn( ({ channelName }: { channelName: string }) => diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index fb570891d93..f1100a82b4b 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -8,7 +8,7 @@ import { noteOpencodeProviderOverrides } from "./doctor-config-analysis.js"; import { runDoctorConfigPreflight } from "./doctor-config-preflight.js"; import { normalizeCompatibilityConfigValues } from "./doctor-legacy-config.js"; import type { DoctorOptions, DoctorPrompter } from "./doctor-prompter.js"; -import { emitDoctorNotes } from "./doctor/emit-notes.js"; +import { emitDoctorNotes, sanitizeDoctorNote } from "./doctor/emit-notes.js"; import { finalizeDoctorConfigFlow } from "./doctor/finalize-config-flow.js"; import { applyLegacyCompatibilityStep, @@ -166,11 +166,12 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { for (const staleCleanup of await channelDoctor.collectChannelDoctorStaleConfigMutations( candidate, + { env: process.env }, )) { if (staleCleanup.changes.length === 0) { continue; } - note(staleCleanup.changes.join("\n"), "Doctor changes"); + note(sanitizeDoctorNote(staleCleanup.changes.join("\n")), "Doctor changes"); ({ cfg, candidate, pendingChanges, fixHints } = applyDoctorConfigMutation({ state: { cfg, candidate, pendingChanges, fixHints }, mutation: staleCleanup, @@ -195,6 +196,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { const repairSequence = await runDoctorRepairSequence({ state: { cfg, candidate, pendingChanges, fixHints }, doctorFixCommand, + env: process.env, }); ({ cfg, candidate, pendingChanges, fixHints } = repairSequence.state); emitDoctorNotes({ @@ -209,6 +211,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { warningNotes: await collectDoctorPreviewWarnings({ cfg: candidate, doctorFixCommand, + env: process.env, }), }); } @@ -216,10 +219,11 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { const mutableAllowlistWarnings = collectMutableAllowlistWarnings ? await collectMutableAllowlistWarnings({ cfg: candidate, + env: process.env, }) : []; if (mutableAllowlistWarnings.length > 0) { - note(mutableAllowlistWarnings.join("\n"), "Doctor warnings"); + note(sanitizeDoctorNote(mutableAllowlistWarnings.join("\n")), "Doctor warnings"); } const unknownStep = applyUnknownConfigKeyStep({ diff --git a/src/commands/doctor/emit-notes.test.ts b/src/commands/doctor/emit-notes.test.ts index 315dfba8877..ecf05ee0b3e 100644 --- a/src/commands/doctor/emit-notes.test.ts +++ b/src/commands/doctor/emit-notes.test.ts @@ -29,6 +29,25 @@ describe("doctor note emission", () => { expect(note.mock.calls).toEqual([["warning only", "Doctor warnings"]]); }); + it("sanitizes emitted notes from plugin-provided doctor output", () => { + const note = vi.fn(); + + emitDoctorNotes({ + note, + changeNotes: ["change \u001B[31mred\u001B[0m\nnext line"], + warningNotes: [ + `warning \u001B]8;;https://example.test\u001B\\link\u001B]8;;\u001B\\${String.fromCharCode( + 0x9b, + )}\r`, + ], + }); + + expect(note.mock.calls).toEqual([ + ["change red\nnext line", "Doctor changes"], + ["warning link", "Doctor warnings"], + ]); + }); + it("emits nothing when note groups are omitted or empty", () => { const note = vi.fn(); diff --git a/src/commands/doctor/emit-notes.ts b/src/commands/doctor/emit-notes.ts index 884bf26c89f..5a51e9a1a0f 100644 --- a/src/commands/doctor/emit-notes.ts +++ b/src/commands/doctor/emit-notes.ts @@ -1,12 +1,21 @@ +import { sanitizeForLog } from "../../terminal/ansi.js"; + +export 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(change, "Doctor changes"); + params.note(sanitizeDoctorNote(change), "Doctor changes"); } for (const warning of params.warningNotes ?? []) { - params.note(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 3277e27ccb8..ad0913a4ccf 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 { @@ -18,6 +18,7 @@ import { maybeRepairStalePluginConfig } from "./shared/stale-plugin-config.js"; export async function runDoctorRepairSequence(params: { state: DoctorConfigMutationState; doctorFixCommand: string; + env?: NodeJS.ProcessEnv; }): Promise<{ state: DoctorConfigMutationState; changeNotes: string[]; @@ -26,6 +27,7 @@ export async function runDoctorRepairSequence(params: { let state = params.state; const changeNotes: string[] = []; const warningNotes: string[] = []; + const env = params.env ?? process.env; const sanitizeLines = (lines: string[]) => lines.map((line) => sanitizeForLog(line)).join("\n"); const applyMutation = (mutation: { @@ -49,17 +51,18 @@ export async function runDoctorRepairSequence(params: { for (const mutation of await collectChannelDoctorRepairMutations({ cfg: state.candidate, doctorFixCommand: params.doctorFixCommand, + env, })) { applyMutation(mutation); } applyMutation(maybeRepairOpenPolicyAllowFrom(state.candidate)); - applyMutation(maybeRepairBundledPluginLoadPaths(state.candidate, process.env)); - applyMutation(maybeRepairStalePluginConfig(state.candidate, process.env)); + applyMutation(maybeRepairBundledPluginLoadPaths(state.candidate, env)); + applyMutation(maybeRepairStalePluginConfig(state.candidate, env)); applyMutation(await maybeRepairAllowlistPolicyAllowFrom(state.candidate)); const emptyAllowlistWarnings = scanEmptyAllowlistPolicyWarnings(state.candidate, { doctorFixCommand: params.doctorFixCommand, - 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 738516c3dc8..ec059c77213 100644 --- a/src/commands/doctor/shared/channel-doctor.test.ts +++ b/src/commands/doctor/shared/channel-doctor.test.ts @@ -1,52 +1,273 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { collectChannelDoctorCompatibilityMutations } from "./channel-doctor.js"; +import { + collectChannelDoctorCompatibilityMutations, + collectChannelDoctorEmptyAllowlistExtraWarnings, + collectChannelDoctorMutableAllowlistWarnings, + collectChannelDoctorStaleConfigMutations, + createChannelDoctorEmptyAllowlistPolicyHooks, +} from "./channel-doctor.js"; const mocks = vi.hoisted(() => ({ - getChannelPlugin: vi.fn(), + getLoadedChannelPlugin: vi.fn(), getBundledChannelPlugin: vi.fn(), - listChannelPlugins: vi.fn(), - listBundledChannelPlugins: vi.fn(), + getBundledChannelSetupPlugin: vi.fn(), + resolveReadOnlyChannelPluginsForConfig: vi.fn(), })); vi.mock("../../../channels/plugins/registry.js", () => ({ - getChannelPlugin: (...args: Parameters) => - mocks.getChannelPlugin(...args), - listChannelPlugins: (...args: Parameters) => - mocks.listChannelPlugins(...args), + getLoadedChannelPlugin: (...args: Parameters) => + mocks.getLoadedChannelPlugin(...args), })); vi.mock("../../../channels/plugins/bundled.js", () => ({ getBundledChannelPlugin: (...args: Parameters) => mocks.getBundledChannelPlugin(...args), - listBundledChannelPlugins: (...args: Parameters) => - mocks.listBundledChannelPlugins(...args), + getBundledChannelSetupPlugin: (...args: Parameters) => + mocks.getBundledChannelSetupPlugin(...args), +})); + +vi.mock("../../../channels/plugins/read-only.js", () => ({ + resolveReadOnlyChannelPluginsForConfig: ( + ...args: Parameters + ) => mocks.resolveReadOnlyChannelPluginsForConfig(...args), })); describe("channel doctor compatibility mutations", () => { beforeEach(() => { - mocks.getChannelPlugin.mockReset(); + mocks.getLoadedChannelPlugin.mockReset(); mocks.getBundledChannelPlugin.mockReset(); - mocks.listChannelPlugins.mockReset(); - mocks.listBundledChannelPlugins.mockReset(); - mocks.getChannelPlugin.mockReturnValue(undefined); + mocks.getBundledChannelSetupPlugin.mockReset(); + mocks.resolveReadOnlyChannelPluginsForConfig.mockReset(); + mocks.getLoadedChannelPlugin.mockReturnValue(undefined); mocks.getBundledChannelPlugin.mockReturnValue(undefined); - mocks.listChannelPlugins.mockReturnValue([]); - mocks.listBundledChannelPlugins.mockReturnValue([]); + mocks.getBundledChannelSetupPlugin.mockReturnValue(undefined); + mocks.resolveReadOnlyChannelPluginsForConfig.mockReturnValue({ plugins: [] }); }); it("skips plugin discovery when no channels are configured", () => { const result = collectChannelDoctorCompatibilityMutations({} as never); expect(result).toEqual([]); - expect(mocks.listChannelPlugins).not.toHaveBeenCalled(); - expect(mocks.listBundledChannelPlugins).not.toHaveBeenCalled(); + expect(mocks.resolveReadOnlyChannelPluginsForConfig).not.toHaveBeenCalled(); }); - it("only evaluates configured channel ids", () => { + it("skips plugin discovery when only channel defaults are configured", async () => { + const result = await collectChannelDoctorStaleConfigMutations({ + channels: { + defaults: { + enabled: true, + }, + }, + } as never); + + expect(result).toEqual([]); + expect(mocks.resolveReadOnlyChannelPluginsForConfig).not.toHaveBeenCalled(); + expect(mocks.getLoadedChannelPlugin).not.toHaveBeenCalled(); + expect(mocks.getBundledChannelSetupPlugin).not.toHaveBeenCalled(); + expect(mocks.getBundledChannelPlugin).not.toHaveBeenCalled(); + }); + + it("skips plugin discovery for explicitly disabled channels", () => { + const result = collectChannelDoctorCompatibilityMutations({ + channels: { + mattermost: { + enabled: false, + }, + }, + } as never); + + expect(result).toEqual([]); + expect(mocks.resolveReadOnlyChannelPluginsForConfig).not.toHaveBeenCalled(); + expect(mocks.getLoadedChannelPlugin).not.toHaveBeenCalled(); + expect(mocks.getBundledChannelSetupPlugin).not.toHaveBeenCalled(); + expect(mocks.getBundledChannelPlugin).not.toHaveBeenCalled(); + }); + + it("uses read-only doctor adapters for configured channel ids", () => { const normalizeCompatibilityConfig = vi.fn(({ cfg }: { cfg: unknown }) => ({ config: cfg, changes: ["matrix"], })); + mocks.resolveReadOnlyChannelPluginsForConfig.mockReturnValue({ + plugins: [ + { + id: "matrix", + doctor: { normalizeCompatibilityConfig }, + }, + ], + }); + + const cfg = { + channels: { + matrix: { + enabled: true, + }, + }, + }; + + const result = collectChannelDoctorCompatibilityMutations(cfg as never); + + expect(result).toHaveLength(1); + expect(normalizeCompatibilityConfig).toHaveBeenCalledTimes(1); + expect(mocks.resolveReadOnlyChannelPluginsForConfig).toHaveBeenCalledWith(cfg, { + includePersistedAuthState: false, + }); + expect(mocks.getLoadedChannelPlugin).toHaveBeenCalledWith("matrix"); + expect(mocks.getBundledChannelSetupPlugin).toHaveBeenCalledWith("matrix"); + expect(mocks.getBundledChannelPlugin).toHaveBeenCalledWith("matrix"); + expect(mocks.getBundledChannelSetupPlugin).not.toHaveBeenCalledWith("discord"); + }); + + it("merges partial doctor adapters instead of masking runtime-only hooks", async () => { + const normalizeCompatibilityConfig = vi.fn(({ cfg }: { cfg: unknown }) => ({ + config: cfg, + changes: ["matrix"], + })); + const collectMutableAllowlistWarnings = vi.fn(() => ["runtime warning"]); + mocks.resolveReadOnlyChannelPluginsForConfig.mockReturnValue({ + plugins: [ + { + id: "matrix", + doctor: { normalizeCompatibilityConfig }, + }, + ], + }); + mocks.getBundledChannelPlugin.mockImplementation((id: string) => + id === "matrix" + ? { + id: "matrix", + doctor: { collectMutableAllowlistWarnings }, + } + : undefined, + ); + + const cfg = { + channels: { + matrix: { + enabled: true, + }, + }, + }; + + expect(collectChannelDoctorCompatibilityMutations(cfg as never)).toHaveLength(1); + await expect( + collectChannelDoctorMutableAllowlistWarnings({ cfg: cfg as never }), + ).resolves.toEqual(["runtime warning"]); + expect(normalizeCompatibilityConfig).toHaveBeenCalledTimes(1); + expect(collectMutableAllowlistWarnings).toHaveBeenCalledTimes(1); + }); + + it("ignores malformed doctor adapter values so valid fallbacks still run", async () => { + const normalizeCompatibilityConfig = vi.fn(({ cfg }: { cfg: unknown }) => ({ + config: cfg, + changes: ["setup"], + })); + const collectMutableAllowlistWarnings = vi.fn(() => ["runtime warning"]); + mocks.resolveReadOnlyChannelPluginsForConfig.mockReturnValue({ + plugins: [ + { + id: "matrix", + doctor: { + normalizeCompatibilityConfig: null, + collectMutableAllowlistWarnings: "not-a-function", + warnOnEmptyGroupSenderAllowlist: "yes", + }, + }, + ], + }); + mocks.getBundledChannelSetupPlugin.mockImplementation((id: string) => + id === "matrix" + ? { + id: "matrix", + doctor: { normalizeCompatibilityConfig }, + } + : undefined, + ); + mocks.getBundledChannelPlugin.mockImplementation((id: string) => + id === "matrix" + ? { + id: "matrix", + doctor: { collectMutableAllowlistWarnings }, + } + : undefined, + ); + + const cfg = { + channels: { + matrix: { + enabled: true, + }, + }, + }; + + expect(collectChannelDoctorCompatibilityMutations(cfg as never)).toHaveLength(1); + await expect( + collectChannelDoctorMutableAllowlistWarnings({ cfg: cfg as never }), + ).resolves.toEqual(["runtime warning"]); + expect(normalizeCompatibilityConfig).toHaveBeenCalledTimes(1); + expect(collectMutableAllowlistWarnings).toHaveBeenCalledTimes(1); + }); + + it("falls back to setup doctor adapters when read-only plugins lack doctor hooks", () => { + const normalizeCompatibilityConfig = vi.fn(({ cfg }: { cfg: unknown }) => ({ + config: cfg, + changes: ["matrix"], + })); + mocks.resolveReadOnlyChannelPluginsForConfig.mockReturnValue({ + plugins: [ + { + id: "matrix", + }, + ], + }); + mocks.getBundledChannelSetupPlugin.mockImplementation((id: string) => + id === "matrix" + ? { + id: "matrix", + doctor: { normalizeCompatibilityConfig }, + } + : undefined, + ); + + const cfg = { + channels: { + matrix: { + enabled: true, + }, + }, + }; + + const result = collectChannelDoctorCompatibilityMutations(cfg as never); + + expect(result).toHaveLength(1); + expect(normalizeCompatibilityConfig).toHaveBeenCalledTimes(1); + expect(mocks.resolveReadOnlyChannelPluginsForConfig).toHaveBeenCalledWith(cfg, { + includePersistedAuthState: false, + }); + expect(mocks.getLoadedChannelPlugin).toHaveBeenCalledWith("matrix"); + expect(mocks.getBundledChannelSetupPlugin).toHaveBeenCalledWith("matrix"); + expect(mocks.getBundledChannelPlugin).toHaveBeenCalledWith("matrix"); + }); + + it("falls back to bundled runtime doctor adapters when setup adapters lack doctor hooks", () => { + const normalizeCompatibilityConfig = vi.fn(({ cfg }: { cfg: unknown }) => ({ + config: cfg, + changes: ["matrix"], + })); + mocks.resolveReadOnlyChannelPluginsForConfig.mockReturnValue({ + plugins: [ + { + id: "matrix", + }, + ], + }); + mocks.getBundledChannelSetupPlugin.mockImplementation((id: string) => + id === "matrix" + ? { + id: "matrix", + } + : undefined, + ); mocks.getBundledChannelPlugin.mockImplementation((id: string) => id === "matrix" ? { @@ -68,9 +289,164 @@ describe("channel doctor compatibility mutations", () => { expect(result).toHaveLength(1); expect(normalizeCompatibilityConfig).toHaveBeenCalledTimes(1); - expect(mocks.getChannelPlugin).toHaveBeenCalledWith("matrix"); + expect(mocks.getLoadedChannelPlugin).toHaveBeenCalledWith("matrix"); + expect(mocks.getBundledChannelSetupPlugin).toHaveBeenCalledWith("matrix"); expect(mocks.getBundledChannelPlugin).toHaveBeenCalledWith("matrix"); - expect(mocks.getBundledChannelPlugin).not.toHaveBeenCalledWith("discord"); - expect(mocks.listBundledChannelPlugins).not.toHaveBeenCalled(); + }); + + it("passes explicit env into read-only channel plugin discovery", () => { + const cfg = { + channels: { + matrix: { + enabled: true, + }, + }, + }; + const env = { OPENCLAW_HOME: "/tmp/openclaw-test-home" }; + + collectChannelDoctorCompatibilityMutations(cfg as never, { env }); + + expect(mocks.resolveReadOnlyChannelPluginsForConfig).toHaveBeenCalledWith(cfg, { + env, + includePersistedAuthState: false, + }); + }); + + it("keeps configured channel doctor lookup non-fatal when setup loading fails", () => { + mocks.resolveReadOnlyChannelPluginsForConfig.mockImplementation(() => { + throw new Error("missing runtime dep"); + }); + mocks.getBundledChannelSetupPlugin.mockImplementation((id: string) => { + if (id === "discord") { + throw new Error("missing runtime dep"); + } + return undefined; + }); + + const result = collectChannelDoctorCompatibilityMutations({ + channels: { + discord: { + enabled: true, + }, + }, + } as never); + + expect(result).toEqual([]); + expect(mocks.getLoadedChannelPlugin).toHaveBeenCalledWith("discord"); + expect(mocks.getBundledChannelSetupPlugin).toHaveBeenCalledWith("discord"); + expect(mocks.getBundledChannelPlugin).toHaveBeenCalledWith("discord"); + }); + + it("uses config for empty allowlist lookup without exposing it to plugin hooks", () => { + const collectEmptyAllowlistExtraWarnings = vi.fn(({ prefix }: { prefix: string }) => [ + `${prefix} extra`, + ]); + const cfg = { + channels: { + matrix: { + groupPolicy: "allowlist", + }, + }, + }; + mocks.resolveReadOnlyChannelPluginsForConfig.mockReturnValue({ + plugins: [ + { + id: "matrix", + doctor: { collectEmptyAllowlistExtraWarnings }, + }, + ], + }); + + const result = collectChannelDoctorEmptyAllowlistExtraWarnings({ + account: {}, + channelName: "matrix", + cfg: cfg as never, + prefix: "channels.matrix", + }); + + expect(result).toEqual(["channels.matrix extra"]); + expect(mocks.resolveReadOnlyChannelPluginsForConfig).toHaveBeenCalledWith(cfg, { + includePersistedAuthState: false, + }); + expect(collectEmptyAllowlistExtraWarnings.mock.calls[0]?.[0]).not.toHaveProperty("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: {}, + }, + }, + slack: { + accounts: { + team: {}, + }, + }, + }, + }; + const env = { OPENCLAW_HOME: "/tmp/openclaw-test-home" }; + mocks.resolveReadOnlyChannelPluginsForConfig.mockReturnValue({ + plugins: [ + { + id: "matrix", + doctor: { + collectEmptyAllowlistExtraWarnings, + shouldSkipDefaultEmptyGroupAllowlistWarning, + }, + }, + { + id: "slack", + doctor: { + collectEmptyAllowlistExtraWarnings, + }, + }, + ], + }); + + const hooks = createChannelDoctorEmptyAllowlistPolicyHooks({ cfg: cfg as never, env }); + + expect( + hooks.extraWarningsForAccount({ + account: {}, + channelName: "matrix", + prefix: "channels.matrix.accounts.work", + }), + ).toEqual(["channels.matrix.accounts.work extra"]); + expect( + hooks.shouldSkipDefaultEmptyGroupAllowlistWarning({ + account: {}, + channelName: "matrix", + prefix: "channels.matrix.accounts.work", + }), + ).toBe(true); + expect( + hooks.extraWarningsForAccount({ + account: {}, + channelName: "matrix", + prefix: "channels.matrix.accounts.personal", + }), + ).toEqual(["channels.matrix.accounts.personal extra"]); + expect( + hooks.extraWarningsForAccount({ + account: {}, + channelName: "slack", + prefix: "channels.slack.accounts.team", + }), + ).toEqual(["channels.slack.accounts.team extra"]); + + expect(mocks.resolveReadOnlyChannelPluginsForConfig).toHaveBeenCalledTimes(1); + expect(mocks.resolveReadOnlyChannelPluginsForConfig).toHaveBeenCalledWith(cfg, { + env, + includePersistedAuthState: false, + }); + expect(collectEmptyAllowlistExtraWarnings).toHaveBeenCalledTimes(3); + expect(shouldSkipDefaultEmptyGroupAllowlistWarning).toHaveBeenCalledTimes(1); }); }); diff --git a/src/commands/doctor/shared/channel-doctor.ts b/src/commands/doctor/shared/channel-doctor.ts index b2084d141c0..75e6ff02423 100644 --- a/src/commands/doctor/shared/channel-doctor.ts +++ b/src/commands/doctor/shared/channel-doctor.ts @@ -1,8 +1,9 @@ import { getBundledChannelPlugin, - listBundledChannelPlugins, + getBundledChannelSetupPlugin, } from "../../../channels/plugins/bundled.js"; -import { getChannelPlugin, listChannelPlugins } from "../../../channels/plugins/registry.js"; +import { resolveReadOnlyChannelPluginsForConfig } from "../../../channels/plugins/read-only.js"; +import { getLoadedChannelPlugin } from "../../../channels/plugins/registry.js"; import type { ChannelDoctorAdapter, ChannelDoctorConfigMutation, @@ -12,10 +13,51 @@ import type { import type { OpenClawConfig } from "../../../config/types.openclaw.js"; type ChannelDoctorEntry = { - channelId: string; doctor: ChannelDoctorAdapter; }; +type ChannelDoctorPluginCandidate = { + id: string; + doctor?: ChannelDoctorAdapter; +}; + +type ChannelDoctorLookupContext = { + cfg: OpenClawConfig; + env?: NodeJS.ProcessEnv; +}; + +type ChannelDoctorEmptyAllowlistLookupParams = ChannelDoctorEmptyAllowlistAccountContext & { + cfg?: OpenClawConfig; +}; + +const channelDoctorFunctionKeys = new Set([ + "normalizeCompatibilityConfig", + "collectPreviewWarnings", + "collectMutableAllowlistWarnings", + "repairConfig", + "runConfigSequence", + "cleanStaleConfig", + "collectEmptyAllowlistExtraWarnings", + "shouldSkipDefaultEmptyGroupAllowlistWarning", +]); + +const channelDoctorBooleanKeys = new Set([ + "groupAllowFromFallbackToAllowFrom", + "warnOnEmptyGroupSenderAllowlist", +]); + +const channelDoctorEnumValues: Partial>> = { + dmAllowFromMode: new Set(["topOnly", "topOrNested", "nestedOnly"]), + groupModel: new Set(["sender", "route", "hybrid"]), +}; + +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) @@ -24,55 +66,195 @@ function collectConfiguredChannelIds(cfg: OpenClawConfig): string[] { if (!channels) { return []; } + const channelEntries = channels as Record; return Object.keys(channels) - .filter((channelId) => channelId !== "defaults") + .filter((channelId) => { + if (channelId === "defaults") { + return false; + } + const entry = channelEntries[channelId]; + return ( + !entry || + typeof entry !== "object" || + Array.isArray(entry) || + (entry as { enabled?: unknown }).enabled !== false + ); + }) .toSorted(); } -function safeListActiveChannelPlugins() { +function safeGetLoadedChannelPlugin(id: string) { try { - return listChannelPlugins(); + return getLoadedChannelPlugin(id); + } catch { + return undefined; + } +} + +function safeGetBundledChannelSetupPlugin(id: string) { + try { + return getBundledChannelSetupPlugin(id); + } catch { + return undefined; + } +} + +function safeGetBundledChannelPlugin(id: string) { + try { + return getBundledChannelPlugin(id); + } catch { + return undefined; + } +} + +function safeListReadOnlyChannelPlugins(context: ChannelDoctorLookupContext) { + try { + return resolveReadOnlyChannelPluginsForConfig(context.cfg, { + ...(context.env ? { env: context.env } : {}), + includePersistedAuthState: false, + }).plugins; } catch { return []; } } -function safeListBundledChannelPlugins() { - try { - return listBundledChannelPlugins(); - } catch { - return []; - } +function listReadOnlyChannelPluginsById( + context: ChannelDoctorLookupContext, +): Map { + return new Map(safeListReadOnlyChannelPlugins(context).map((plugin) => [plugin.id, plugin])); } -function listChannelDoctorEntries(channelIds?: readonly string[]): ChannelDoctorEntry[] { - const byId = new Map(); - const selectedIds = channelIds ? new Set(channelIds) : null; - const plugins = selectedIds - ? [...selectedIds].flatMap((id) => { - let activeOrBundledPlugin; - try { - activeOrBundledPlugin = getChannelPlugin(id); - } catch { - activeOrBundledPlugin = undefined; - } - if (activeOrBundledPlugin?.doctor) { - return [activeOrBundledPlugin]; - } - const bundledPlugin = getBundledChannelPlugin(id); - return bundledPlugin ? [bundledPlugin] : []; - }) - : [...safeListActiveChannelPlugins(), ...safeListBundledChannelPlugins()]; - for (const plugin of plugins) { - if (!plugin.doctor) { +function mergeDoctorAdapters( + adapters: Array, +): ChannelDoctorAdapter | undefined { + const merged: Partial> = {}; + for (const adapter of adapters) { + if (!adapter) { continue; } - const existing = byId.get(plugin.id); - if (!existing) { - byId.set(plugin.id, { channelId: plugin.id, doctor: plugin.doctor }); + for (const [key, value] of Object.entries(adapter) as Array< + [keyof ChannelDoctorAdapter, unknown] + >) { + if (merged[key] !== undefined) { + continue; + } + if (!isValidChannelDoctorAdapterValue(key, value)) { + continue; + } + merged[key] = value; } } - return [...byId.values()]; + return Object.keys(merged).length > 0 ? (merged as ChannelDoctorAdapter) : undefined; +} + +function isValidChannelDoctorAdapterValue( + key: keyof ChannelDoctorAdapter, + value: unknown, +): boolean { + if (value == null) { + return false; + } + if (channelDoctorFunctionKeys.has(key)) { + return typeof value === "function"; + } + if (channelDoctorBooleanKeys.has(key)) { + return typeof value === "boolean"; + } + const enumValues = channelDoctorEnumValues[key]; + if (enumValues) { + return typeof value === "string" && enumValues.has(value); + } + if (key === "legacyConfigRules") { + return Array.isArray(value); + } + return false; +} + +function listChannelDoctorEntries( + channelIds: readonly string[], + context: ChannelDoctorLookupContext, + options: { + readOnlyPluginsById?: ReadonlyMap; + } = {}, +): ChannelDoctorEntry[] { + if (channelIds.length === 0) { + return []; + } + const selectedIds = new Set(channelIds); + const readOnlyPluginsById = + options.readOnlyPluginsById ?? listReadOnlyChannelPluginsById(context); + + const entries: ChannelDoctorEntry[] = []; + for (const id of selectedIds) { + const doctor = mergeDoctorAdapters([ + readOnlyPluginsById.get(id)?.doctor, + safeGetLoadedChannelPlugin(id)?.doctor, + safeGetBundledChannelSetupPlugin(id)?.doctor, + safeGetBundledChannelPlugin(id)?.doctor, + ]); + if (!doctor) { + continue; + } + entries.push({ doctor }); + } + return entries; +} + +function toPluginEmptyAllowlistContext({ + cfg: _cfg, + ...params +}: ChannelDoctorEmptyAllowlistLookupParams): ChannelDoctorEmptyAllowlistAccountContext { + return params; +} + +function collectEmptyAllowlistExtraWarningsForEntries( + entries: readonly ChannelDoctorEntry[], + params: ChannelDoctorEmptyAllowlistLookupParams, +): string[] { + const warnings: string[] = []; + const pluginParams = toPluginEmptyAllowlistContext(params); + for (const entry of entries) { + const lines = entry.doctor.collectEmptyAllowlistExtraWarnings?.(pluginParams); + if (lines?.length) { + warnings.push(...lines); + } + } + return warnings; +} + +function shouldSkipDefaultEmptyGroupAllowlistWarningForEntries( + entries: readonly ChannelDoctorEntry[], + params: ChannelDoctorEmptyAllowlistLookupParams, +): boolean { + const pluginParams = toPluginEmptyAllowlistContext(params); + return entries.some( + (entry) => entry.doctor.shouldSkipDefaultEmptyGroupAllowlistWarning?.(pluginParams) === true, + ); +} + +export function createChannelDoctorEmptyAllowlistPolicyHooks( + context: ChannelDoctorLookupContext, +): ChannelDoctorEmptyAllowlistPolicyHooks { + const readOnlyPluginsById = listReadOnlyChannelPluginsById(context); + const entriesByChannel = new Map(); + const entriesForChannel = (channelName: string) => { + const existing = entriesByChannel.get(channelName); + if (existing) { + return existing; + } + const entries = listChannelDoctorEntries([channelName], context, { readOnlyPluginsById }); + 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: { @@ -82,7 +264,10 @@ export async function runChannelDoctorConfigSequences(params: { }): Promise { const changeNotes: string[] = []; const warningNotes: string[] = []; - for (const entry of listChannelDoctorEntries(collectConfiguredChannelIds(params.cfg))) { + for (const entry of listChannelDoctorEntries(collectConfiguredChannelIds(params.cfg), { + cfg: params.cfg, + env: params.env, + })) { const result = await entry.doctor.runConfigSequence?.(params); if (!result) { continue; @@ -95,6 +280,7 @@ export async function runChannelDoctorConfigSequences(params: { export function collectChannelDoctorCompatibilityMutations( cfg: OpenClawConfig, + options: { env?: NodeJS.ProcessEnv } = {}, ): ChannelDoctorConfigMutation[] { const channelIds = collectConfiguredChannelIds(cfg); if (channelIds.length === 0) { @@ -102,7 +288,7 @@ export function collectChannelDoctorCompatibilityMutations( } const mutations: ChannelDoctorConfigMutation[] = []; let nextCfg = cfg; - for (const entry of listChannelDoctorEntries(channelIds)) { + for (const entry of listChannelDoctorEntries(channelIds, { cfg, env: options.env })) { const mutation = entry.doctor.normalizeCompatibilityConfig?.({ cfg: nextCfg }); if (!mutation || mutation.changes.length === 0) { continue; @@ -115,10 +301,14 @@ export function collectChannelDoctorCompatibilityMutations( export async function collectChannelDoctorStaleConfigMutations( cfg: OpenClawConfig, + options: { env?: NodeJS.ProcessEnv } = {}, ): Promise { const mutations: ChannelDoctorConfigMutation[] = []; let nextCfg = cfg; - for (const entry of listChannelDoctorEntries(collectConfiguredChannelIds(cfg))) { + for (const entry of listChannelDoctorEntries(collectConfiguredChannelIds(cfg), { + cfg, + env: options.env, + })) { const mutation = await entry.doctor.cleanStaleConfig?.({ cfg: nextCfg }); if (!mutation || mutation.changes.length === 0) { continue; @@ -132,9 +322,13 @@ export async function collectChannelDoctorStaleConfigMutations( export async function collectChannelDoctorPreviewWarnings(params: { cfg: OpenClawConfig; doctorFixCommand: string; + env?: NodeJS.ProcessEnv; }): Promise { const warnings: string[] = []; - for (const entry of listChannelDoctorEntries(collectConfiguredChannelIds(params.cfg))) { + for (const entry of listChannelDoctorEntries(collectConfiguredChannelIds(params.cfg), { + cfg: params.cfg, + env: params.env, + })) { const lines = await entry.doctor.collectPreviewWarnings?.(params); if (lines?.length) { warnings.push(...lines); @@ -145,9 +339,13 @@ export async function collectChannelDoctorPreviewWarnings(params: { export async function collectChannelDoctorMutableAllowlistWarnings(params: { cfg: OpenClawConfig; + env?: NodeJS.ProcessEnv; }): Promise { const warnings: string[] = []; - for (const entry of listChannelDoctorEntries(collectConfiguredChannelIds(params.cfg))) { + for (const entry of listChannelDoctorEntries(collectConfiguredChannelIds(params.cfg), { + cfg: params.cfg, + env: params.env, + })) { const lines = await entry.doctor.collectMutableAllowlistWarnings?.(params); if (lines?.length) { warnings.push(...lines); @@ -159,10 +357,14 @@ export async function collectChannelDoctorMutableAllowlistWarnings(params: { export async function collectChannelDoctorRepairMutations(params: { cfg: OpenClawConfig; doctorFixCommand: string; + env?: NodeJS.ProcessEnv; }): Promise { const mutations: ChannelDoctorConfigMutation[] = []; let nextCfg = params.cfg; - for (const entry of listChannelDoctorEntries(collectConfiguredChannelIds(params.cfg))) { + for (const entry of listChannelDoctorEntries(collectConfiguredChannelIds(params.cfg), { + cfg: params.cfg, + env: params.env, + })) { const mutation = await entry.doctor.repairConfig?.({ cfg: nextCfg, doctorFixCommand: params.doctorFixCommand, @@ -180,22 +382,23 @@ export async function collectChannelDoctorRepairMutations(params: { } export function collectChannelDoctorEmptyAllowlistExtraWarnings( - params: ChannelDoctorEmptyAllowlistAccountContext, + params: ChannelDoctorEmptyAllowlistLookupParams, ): string[] { - const warnings: string[] = []; - for (const entry of listChannelDoctorEntries([params.channelName])) { - const lines = entry.doctor.collectEmptyAllowlistExtraWarnings?.(params); - if (lines?.length) { - warnings.push(...lines); - } - } - return warnings; + return collectEmptyAllowlistExtraWarningsForEntries( + listChannelDoctorEntries([params.channelName], { + cfg: params.cfg ?? {}, + }), + params, + ); } export function shouldSkipChannelDoctorDefaultEmptyGroupAllowlistWarning( - params: ChannelDoctorEmptyAllowlistAccountContext, + params: ChannelDoctorEmptyAllowlistLookupParams, ): boolean { - return listChannelDoctorEntries([params.channelName]).some( - (entry) => entry.doctor.shouldSkipDefaultEmptyGroupAllowlistWarning?.(params) === true, + return shouldSkipDefaultEmptyGroupAllowlistWarningForEntries( + listChannelDoctorEntries([params.channelName], { + cfg: params.cfg ?? {}, + }), + params, ); } diff --git a/src/commands/doctor/shared/empty-allowlist-policy.ts b/src/commands/doctor/shared/empty-allowlist-policy.ts index b4f678fd0cf..b9c486df30a 100644 --- a/src/commands/doctor/shared/empty-allowlist-policy.ts +++ b/src/commands/doctor/shared/empty-allowlist-policy.ts @@ -1,3 +1,4 @@ +import type { OpenClawConfig } from "../../../config/types.openclaw.js"; import { getDoctorChannelCapabilities } from "../channel-capabilities.js"; import type { DoctorAccountRecord, DoctorAllowFromList } from "../types.js"; import { hasAllowFromEntries } from "./allowlist.js"; @@ -6,9 +7,11 @@ import { shouldSkipChannelDoctorDefaultEmptyGroupAllowlistWarning } from "./chan type CollectEmptyAllowlistPolicyWarningsParams = { account: DoctorAccountRecord; channelName?: string; + cfg?: OpenClawConfig; doctorFixCommand: string; parent?: DoctorAccountRecord; prefix: string; + shouldSkipDefaultEmptyGroupAllowlistWarning?: typeof shouldSkipChannelDoctorDefaultEmptyGroupAllowlistWarning; }; function usesSenderBasedGroupAllowlist(channelName?: string): boolean { @@ -64,9 +67,13 @@ export function collectEmptyAllowlistPolicyWarningsForAccount( if ( params.channelName && - shouldSkipChannelDoctorDefaultEmptyGroupAllowlistWarning({ + ( + params.shouldSkipDefaultEmptyGroupAllowlistWarning ?? + shouldSkipChannelDoctorDefaultEmptyGroupAllowlistWarning + )({ account: params.account, channelName: params.channelName, + cfg: params.cfg, dmPolicy, effectiveAllowFrom, parent: params.parent, diff --git a/src/commands/doctor/shared/empty-allowlist-scan.ts b/src/commands/doctor/shared/empty-allowlist-scan.ts index d5924936d69..1c71d106333 100644 --- a/src/commands/doctor/shared/empty-allowlist-scan.ts +++ b/src/commands/doctor/shared/empty-allowlist-scan.ts @@ -1,20 +1,15 @@ +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"; import { asObjectRecord } from "./object.js"; -export type EmptyAllowlistAccountScanParams = { - account: DoctorAccountRecord; - channelName: string; - dmPolicy?: string; - effectiveAllowFrom?: DoctorAllowFromList; - parent?: DoctorAccountRecord; - prefix: string; -}; - type ScanEmptyAllowlistPolicyWarningsParams = { doctorFixCommand: string; - extraWarningsForAccount?: (params: EmptyAllowlistAccountScanParams) => string[]; + extraWarningsForAccount?: (params: ChannelDoctorEmptyAllowlistAccountContext) => string[]; + shouldSkipDefaultEmptyGroupAllowlistWarning?: ( + params: ChannelDoctorEmptyAllowlistAccountContext, + ) => boolean; }; export function scanEmptyAllowlistPolicyWarnings( @@ -53,9 +48,12 @@ export function scanEmptyAllowlistPolicyWarnings( ...collectEmptyAllowlistPolicyWarningsForAccount({ account, channelName, + cfg, doctorFixCommand: params.doctorFixCommand, 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 934c88ce819..8899304ec64 100644 --- a/src/commands/doctor/shared/preview-warnings.ts +++ b/src/commands/doctor/shared/preview-warnings.ts @@ -78,8 +78,10 @@ function hasConfiguredSafeBins(cfg: OpenClawConfig): boolean { export async function collectDoctorPreviewWarnings(params: { cfg: OpenClawConfig; doctorFixCommand: string; + env?: NodeJS.ProcessEnv; }): Promise { const warnings: string[] = []; + const env = params.env ?? process.env; const hasChannelConfig = hasChannels(params.cfg); const hasPluginConfig = hasPlugins(params.cfg); @@ -88,7 +90,7 @@ export async function collectDoctorPreviewWarnings(params: { ? await import("./channel-plugin-blockers.js") : undefined; const channelPluginBlockerHits = - channelPluginRuntime?.scanConfiguredChannelPluginBlockers(params.cfg, process.env) ?? []; + channelPluginRuntime?.scanConfiguredChannelPluginBlockers(params.cfg, env) ?? []; if (channelPluginRuntime && channelPluginBlockerHits.length > 0) { warnings.push( channelPluginRuntime @@ -102,6 +104,7 @@ export async function collectDoctorPreviewWarnings(params: { const channelDoctorWarnings = await collectChannelDoctorPreviewWarnings({ cfg: params.cfg, doctorFixCommand: params.doctorFixCommand, + env, }); if (channelDoctorWarnings.length > 0) { warnings.push(...channelDoctorWarnings); @@ -126,13 +129,13 @@ export async function collectDoctorPreviewWarnings(params: { isStalePluginAutoRepairBlocked, scanStalePluginConfig, } = await import("./stale-plugin-config.js"); - const stalePluginHits = scanStalePluginConfig(params.cfg, process.env); + const stalePluginHits = scanStalePluginConfig(params.cfg, env); if (stalePluginHits.length > 0) { warnings.push( collectStalePluginConfigWarnings({ hits: stalePluginHits, doctorFixCommand: params.doctorFixCommand, - autoRepairBlocked: isStalePluginAutoRepairBlocked(params.cfg, process.env), + autoRepairBlocked: isStalePluginAutoRepairBlocked(params.cfg, env), }).join("\n"), ); } @@ -141,7 +144,7 @@ export async function collectDoctorPreviewWarnings(params: { if (hasPluginLoadPaths(params.cfg)) { const { collectBundledPluginLoadPathWarnings, scanBundledPluginLoadPathMigrations } = await import("./bundled-plugin-load-paths.js"); - const bundledPluginLoadPathHits = scanBundledPluginLoadPathMigrations(params.cfg, process.env); + const bundledPluginLoadPathHits = scanBundledPluginLoadPathMigrations(params.cfg, env); if (bundledPluginLoadPathHits.length > 0) { warnings.push( collectBundledPluginLoadPathWarnings({ @@ -153,11 +156,17 @@ 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, - extraWarningsForAccount: collectChannelDoctorEmptyAllowlistExtraWarnings, + extraWarningsForAccount: emptyAllowlistHooks.extraWarningsForAccount, + shouldSkipDefaultEmptyGroupAllowlistWarning: + emptyAllowlistHooks.shouldSkipDefaultEmptyGroupAllowlistWarning, }).filter( (warning) => !( diff --git a/src/terminal/ansi.test.ts b/src/terminal/ansi.test.ts index 0032c5f84ad..04745cc835d 100644 --- a/src/terminal/ansi.test.ts +++ b/src/terminal/ansi.test.ts @@ -15,8 +15,10 @@ describe("terminal ansi helpers", () => { "next" + String.fromCharCode(0) + "line" + - String.fromCharCode(127); - expect(sanitizeForLog(input)).toBe("warnnextline"); + String.fromCharCode(127) + + String.fromCharCode(0x9b) + + "done"; + expect(sanitizeForLog(input)).toBe("warnnextlinedone"); }); it("measures wide graphemes by terminal cell width", () => { diff --git a/src/terminal/ansi.ts b/src/terminal/ansi.ts index 2b653b94231..3c6ffc57144 100644 --- a/src/terminal/ansi.ts +++ b/src/terminal/ansi.ts @@ -30,8 +30,8 @@ export function splitGraphemes(input: string): string[] { /** * Sanitize a value for safe interpolation into log messages. - * Strips ANSI escape sequences, C0 control characters (U+0000–U+001F), - * and DEL (U+007F) to prevent log forging / terminal escape injection (CWE-117). + * Strips ANSI escape sequences, C0/C1 control characters, and DEL to + * prevent log forging / terminal escape injection (CWE-117). */ export function sanitizeForLog(v: string): string { // Pattern built at runtime so the source file stays free of literal control @@ -39,7 +39,9 @@ export function sanitizeForLog(v: string): string { const c0Start = String.fromCharCode(0x00); const c0End = String.fromCharCode(0x1f); const del = String.fromCharCode(0x7f); - const controlCharsRegex = new RegExp(`[${c0Start}-${c0End}${del}]`, "g"); + const c1Start = String.fromCharCode(0x80); + const c1End = String.fromCharCode(0x9f); + const controlCharsRegex = new RegExp(`[${c0Start}-${c0End}${del}${c1Start}-${c1End}]`, "g"); return stripAnsi(v).replace(controlCharsRegex, ""); }