From 66f814a0af644e0b282b9d5c523ebe5f8737fe0b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 14:05:46 +0000 Subject: [PATCH] refactor(channels): dedupe plugin routing and channel helpers --- src/channels/ack-reactions.test.ts | 46 +- src/channels/dock.ts | 121 +-- src/channels/mention-gating.test.ts | 34 +- src/channels/plugins/actions/actions.test.ts | 145 ++-- src/channels/plugins/actions/discord.ts | 56 +- src/channels/plugins/actions/shared.ts | 19 + src/channels/plugins/actions/signal.ts | 44 +- src/channels/plugins/actions/telegram.ts | 79 +- src/channels/plugins/directory-config.ts | 120 ++- src/channels/plugins/group-mentions.test.ts | 157 +++- src/channels/plugins/group-mentions.ts | 245 +++--- src/channels/plugins/load.ts | 27 +- .../plugins/message-actions.security.test.ts | 22 +- src/channels/plugins/message-actions.test.ts | 87 ++ src/channels/plugins/message-actions.ts | 52 +- src/channels/plugins/normalize/imessage.ts | 16 +- src/channels/plugins/normalize/shared.ts | 22 + .../plugins/normalize/targets.test.ts | 32 + src/channels/plugins/normalize/whatsapp.ts | 18 +- .../channel-access-configure.test.ts | 142 ++++ .../onboarding/channel-access-configure.ts | 41 + src/channels/plugins/onboarding/discord.ts | 343 +++----- .../plugins/onboarding/helpers.test.ts | 741 +++++++++++++++++- src/channels/plugins/onboarding/helpers.ts | 443 ++++++++++- .../plugins/onboarding/imessage.test.ts | 24 + src/channels/plugins/onboarding/imessage.ts | 158 ++-- .../plugins/onboarding/signal.test.ts | 39 + src/channels/plugins/onboarding/signal.ts | 168 ++-- src/channels/plugins/onboarding/slack.ts | 277 ++----- .../plugins/onboarding/telegram.test.ts | 23 + src/channels/plugins/onboarding/telegram.ts | 261 ++---- .../plugins/onboarding/whatsapp.test.ts | 134 ++-- .../plugins/outbound/direct-text-media.ts | 119 +++ src/channels/plugins/outbound/discord.test.ts | 66 +- src/channels/plugins/outbound/imessage.ts | 66 +- src/channels/plugins/outbound/load.ts | 30 +- src/channels/plugins/outbound/signal.test.ts | 70 ++ src/channels/plugins/outbound/signal.ts | 61 +- src/channels/plugins/outbound/slack.test.ts | 108 ++- .../plugins/outbound/telegram.test.ts | 116 +++ src/channels/plugins/outbound/telegram.ts | 80 +- src/channels/plugins/plugins-channel.test.ts | 17 +- src/channels/plugins/plugins-core.test.ts | 212 ++--- src/channels/plugins/registry-loader.ts | 35 + .../plugins/status-issues/bluebubbles.test.ts | 66 ++ .../plugins/status-issues/bluebubbles.ts | 102 ++- src/channels/plugins/status-issues/shared.ts | 20 + .../plugins/status-issues/whatsapp.test.ts | 56 ++ .../plugins/status-issues/whatsapp.ts | 71 +- src/channels/plugins/types.adapters.ts | 70 +- .../plugins/whatsapp-heartbeat.test.ts | 17 +- src/channels/sender-label.test.ts | 45 ++ src/channels/sender-label.ts | 22 +- src/channels/status-reactions.test.ts | 73 +- src/channels/status-reactions.ts | 21 +- src/slack/channel-migration.test.ts | 96 +-- src/telegram/group-migration.test.ts | 96 +-- 57 files changed, 3744 insertions(+), 2127 deletions(-) create mode 100644 src/channels/plugins/actions/shared.ts create mode 100644 src/channels/plugins/message-actions.test.ts create mode 100644 src/channels/plugins/normalize/shared.ts create mode 100644 src/channels/plugins/normalize/targets.test.ts create mode 100644 src/channels/plugins/onboarding/channel-access-configure.test.ts create mode 100644 src/channels/plugins/onboarding/channel-access-configure.ts create mode 100644 src/channels/plugins/onboarding/imessage.test.ts create mode 100644 src/channels/plugins/onboarding/signal.test.ts create mode 100644 src/channels/plugins/onboarding/telegram.test.ts create mode 100644 src/channels/plugins/outbound/direct-text-media.ts create mode 100644 src/channels/plugins/outbound/signal.test.ts create mode 100644 src/channels/plugins/outbound/telegram.test.ts create mode 100644 src/channels/plugins/registry-loader.ts create mode 100644 src/channels/plugins/status-issues/bluebubbles.test.ts create mode 100644 src/channels/plugins/status-issues/whatsapp.test.ts create mode 100644 src/channels/sender-label.test.ts diff --git a/src/channels/ack-reactions.test.ts b/src/channels/ack-reactions.test.ts index 738917208..e964a895e 100644 --- a/src/channels/ack-reactions.test.ts +++ b/src/channels/ack-reactions.test.ts @@ -65,62 +65,46 @@ describe("shouldAckReaction", () => { }); it("requires mention gating for group-mentions", () => { + const groupMentionsScope = { + scope: "group-mentions" as const, + isDirect: false, + isGroup: true, + isMentionableGroup: true, + requireMention: true, + canDetectMention: true, + effectiveWasMentioned: true, + }; + expect( shouldAckReaction({ - scope: "group-mentions", - isDirect: false, - isGroup: true, - isMentionableGroup: true, + ...groupMentionsScope, requireMention: false, - canDetectMention: true, - effectiveWasMentioned: true, }), ).toBe(false); expect( shouldAckReaction({ - scope: "group-mentions", - isDirect: false, - isGroup: true, - isMentionableGroup: true, - requireMention: true, + ...groupMentionsScope, canDetectMention: false, - effectiveWasMentioned: true, }), ).toBe(false); expect( shouldAckReaction({ - scope: "group-mentions", - isDirect: false, - isGroup: true, + ...groupMentionsScope, isMentionableGroup: false, - requireMention: true, - canDetectMention: true, - effectiveWasMentioned: true, }), ).toBe(false); expect( shouldAckReaction({ - scope: "group-mentions", - isDirect: false, - isGroup: true, - isMentionableGroup: true, - requireMention: true, - canDetectMention: true, - effectiveWasMentioned: true, + ...groupMentionsScope, }), ).toBe(true); expect( shouldAckReaction({ - scope: "group-mentions", - isDirect: false, - isGroup: true, - isMentionableGroup: true, - requireMention: true, - canDetectMention: true, + ...groupMentionsScope, effectiveWasMentioned: false, shouldBypassMention: true, }), diff --git a/src/channels/dock.ts b/src/channels/dock.ts index df7dcbfe7..2e287aa79 100644 --- a/src/channels/dock.ts +++ b/src/channels/dock.ts @@ -82,6 +82,25 @@ const stringifyAllowFrom = (allowFrom: Array) => const trimAllowFromEntries = (allowFrom: Array) => allowFrom.map((entry) => String(entry).trim()).filter(Boolean); +const DEFAULT_OUTBOUND_TEXT_CHUNK_LIMIT_4000 = { textChunkLimit: 4000 }; + +const DEFAULT_BLOCK_STREAMING_COALESCE = { + blockStreamingCoalesceDefaults: { minChars: 1500, idleMs: 1000 }, +}; + +function formatAllowFromWithReplacements( + allowFrom: Array, + replacements: RegExp[], +): string[] { + return trimAllowFromEntries(allowFrom).map((entry) => { + let normalized = entry; + for (const replacement of replacements) { + normalized = normalized.replace(replacement, ""); + } + return normalized.toLowerCase(); + }); +} + const formatDiscordAllowFrom = (allowFrom: Array) => allowFrom .map((entry) => @@ -168,6 +187,35 @@ function resolveDefaultToCaseInsensitiveAccount(params: { const account = resolveCaseInsensitiveAccount(params.channel?.accounts, params.accountId); return (account?.defaultTo ?? params.channel?.defaultTo)?.trim() || undefined; } + +function resolveChannelDefaultTo( + channel: + | { + accounts?: Record; + defaultTo?: string; + } + | undefined, + accountId?: string | null, +): string | undefined { + return resolveDefaultToCaseInsensitiveAccount({ channel, accountId }); +} + +type CaseInsensitiveDefaultToChannel = { + accounts?: Record; + defaultTo?: string; +}; + +type CaseInsensitiveDefaultToChannels = Partial< + Record<"irc" | "googlechat", CaseInsensitiveDefaultToChannel> +>; + +function resolveNamedChannelDefaultTo(params: { + channels?: CaseInsensitiveDefaultToChannels; + channelId: keyof CaseInsensitiveDefaultToChannels; + accountId?: string | null; +}): string | undefined { + return resolveChannelDefaultTo(params.channels?.[params.channelId], params.accountId); +} // Channel docks: lightweight channel metadata/behavior for shared code paths. // // Rules: @@ -186,7 +234,7 @@ const DOCKS: Record = { nativeCommands: true, blockStreaming: true, }, - outbound: { textChunkLimit: 4000 }, + outbound: DEFAULT_OUTBOUND_TEXT_CHUNK_LIMIT_4000, config: { resolveAllowFrom: ({ cfg, accountId }) => stringifyAllowFrom(resolveTelegramAccount({ cfg, accountId }).config.allowFrom ?? []), @@ -221,7 +269,7 @@ const DOCKS: Record = { enforceOwnerForCommands: true, skipWhenConfigEmpty: true, }, - outbound: { textChunkLimit: 4000 }, + outbound: DEFAULT_OUTBOUND_TEXT_CHUNK_LIMIT_4000, config: { resolveAllowFrom: ({ cfg, accountId }) => resolveWhatsAppAccount({ cfg, accountId }).allowFrom ?? [], @@ -276,9 +324,7 @@ const DOCKS: Record = { threads: true, }, outbound: { textChunkLimit: 2000 }, - streaming: { - blockStreamingCoalesceDefaults: { minChars: 1500, idleMs: 1000 }, - }, + streaming: DEFAULT_BLOCK_STREAMING_COALESCE, elevated: { allowFromFallback: ({ cfg }) => cfg.channels?.discord?.allowFrom ?? cfg.channels?.discord?.dm?.allowFrom, @@ -328,21 +374,13 @@ const DOCKS: Record = { return (account?.allowFrom ?? channel?.allowFrom ?? []).map((entry) => String(entry)); }, formatAllowFrom: ({ allowFrom }) => - allowFrom - .map((entry) => String(entry).trim()) - .filter(Boolean) - .map((entry) => - entry - .replace(/^irc:/i, "") - .replace(/^user:/i, "") - .toLowerCase(), - ), - resolveDefaultTo: ({ cfg, accountId }) => { - const channel = cfg.channels?.irc as - | { accounts?: Record; defaultTo?: string } - | undefined; - return resolveDefaultToCaseInsensitiveAccount({ channel, accountId }); - }, + formatAllowFromWithReplacements(allowFrom, [/^irc:/i, /^user:/i]), + resolveDefaultTo: ({ cfg, accountId }) => + resolveNamedChannelDefaultTo({ + channels: cfg.channels as CaseInsensitiveDefaultToChannels | undefined, + channelId: "irc", + accountId, + }), }, groups: { resolveRequireMention: ({ cfg, accountId, groupId }) => { @@ -385,7 +423,7 @@ const DOCKS: Record = { threads: true, blockStreaming: true, }, - outbound: { textChunkLimit: 4000 }, + outbound: DEFAULT_OUTBOUND_TEXT_CHUNK_LIMIT_4000, config: { resolveAllowFrom: ({ cfg, accountId }) => { const channel = cfg.channels?.googlechat as @@ -400,22 +438,17 @@ const DOCKS: Record = { ); }, formatAllowFrom: ({ allowFrom }) => - allowFrom - .map((entry) => String(entry).trim()) - .filter(Boolean) - .map((entry) => - entry - .replace(/^(googlechat|google-chat|gchat):/i, "") - .replace(/^user:/i, "") - .replace(/^users\//i, "") - .toLowerCase(), - ), - resolveDefaultTo: ({ cfg, accountId }) => { - const channel = cfg.channels?.googlechat as - | { accounts?: Record; defaultTo?: string } - | undefined; - return resolveDefaultToCaseInsensitiveAccount({ channel, accountId }); - }, + formatAllowFromWithReplacements(allowFrom, [ + /^(googlechat|google-chat|gchat):/i, + /^user:/i, + /^users\//i, + ]), + resolveDefaultTo: ({ cfg, accountId }) => + resolveNamedChannelDefaultTo({ + channels: cfg.channels as CaseInsensitiveDefaultToChannels | undefined, + channelId: "googlechat", + accountId, + }), }, groups: { resolveRequireMention: resolveGoogleChatGroupRequireMention, @@ -436,10 +469,8 @@ const DOCKS: Record = { nativeCommands: true, threads: true, }, - outbound: { textChunkLimit: 4000 }, - streaming: { - blockStreamingCoalesceDefaults: { minChars: 1500, idleMs: 1000 }, - }, + outbound: DEFAULT_OUTBOUND_TEXT_CHUNK_LIMIT_4000, + streaming: DEFAULT_BLOCK_STREAMING_COALESCE, config: { resolveAllowFrom: ({ cfg, accountId }) => { const account = resolveSlackAccount({ cfg, accountId }); @@ -472,10 +503,8 @@ const DOCKS: Record = { reactions: true, media: true, }, - outbound: { textChunkLimit: 4000 }, - streaming: { - blockStreamingCoalesceDefaults: { minChars: 1500, idleMs: 1000 }, - }, + outbound: DEFAULT_OUTBOUND_TEXT_CHUNK_LIMIT_4000, + streaming: DEFAULT_BLOCK_STREAMING_COALESCE, config: { resolveAllowFrom: ({ cfg, accountId }) => stringifyAllowFrom(resolveSignalAccount({ cfg, accountId }).config.allowFrom ?? []), @@ -498,7 +527,7 @@ const DOCKS: Record = { reactions: true, media: true, }, - outbound: { textChunkLimit: 4000 }, + outbound: DEFAULT_OUTBOUND_TEXT_CHUNK_LIMIT_4000, config: { resolveAllowFrom: ({ cfg, accountId }) => (resolveIMessageAccount({ cfg, accountId }).config.allowFrom ?? []).map((entry) => diff --git a/src/channels/mention-gating.test.ts b/src/channels/mention-gating.test.ts index e4c7c54ab..c0237a37b 100644 --- a/src/channels/mention-gating.test.ts +++ b/src/channels/mention-gating.test.ts @@ -37,22 +37,20 @@ describe("resolveMentionGating", () => { }); describe("resolveMentionGatingWithBypass", () => { - it("enables bypass when control commands are authorized", () => { - const res = resolveMentionGatingWithBypass({ - isGroup: true, - requireMention: true, - canDetectMention: true, - wasMentioned: false, - hasAnyMention: false, - allowTextCommands: true, - hasControlCommand: true, + it.each([ + { + name: "enables bypass when control commands are authorized", commandAuthorized: true, - }); - expect(res.shouldBypassMention).toBe(true); - expect(res.shouldSkip).toBe(false); - }); - - it("does not bypass when control commands are not authorized", () => { + shouldBypassMention: true, + shouldSkip: false, + }, + { + name: "does not bypass when control commands are not authorized", + commandAuthorized: false, + shouldBypassMention: false, + shouldSkip: true, + }, + ])("$name", ({ commandAuthorized, shouldBypassMention, shouldSkip }) => { const res = resolveMentionGatingWithBypass({ isGroup: true, requireMention: true, @@ -61,9 +59,9 @@ describe("resolveMentionGatingWithBypass", () => { hasAnyMention: false, allowTextCommands: true, hasControlCommand: true, - commandAuthorized: false, + commandAuthorized, }); - expect(res.shouldBypassMention).toBe(false); - expect(res.shouldSkip).toBe(true); + expect(res.shouldBypassMention).toBe(shouldBypassMention); + expect(res.shouldSkip).toBe(shouldSkip); }); }); diff --git a/src/channels/plugins/actions/actions.test.ts b/src/channels/plugins/actions/actions.test.ts index 3c322bcf9..26973f835 100644 --- a/src/channels/plugins/actions/actions.test.ts +++ b/src/channels/plugins/actions/actions.test.ts @@ -114,6 +114,65 @@ function expectModerationActions(actions: string[]) { expect(actions).toContain("ban"); } +function expectChannelCreateAction(actions: string[], expected: boolean) { + if (expected) { + expect(actions).toContain("channel-create"); + return; + } + expect(actions).not.toContain("channel-create"); +} + +function createSignalAccountOverrideCfg(): OpenClawConfig { + return { + channels: { + signal: { + actions: { reactions: false }, + accounts: { + work: { account: "+15550001111", actions: { reactions: true } }, + }, + }, + }, + } as OpenClawConfig; +} + +function createDiscordModerationOverrideCfg(params?: { + channelsEnabled?: boolean; +}): OpenClawConfig { + const accountActions = params?.channelsEnabled + ? { moderation: true, channels: true } + : { moderation: true }; + return { + channels: { + discord: { + actions: { channels: false }, + accounts: { + vime: { token: "d1", actions: accountActions }, + }, + }, + }, + } as OpenClawConfig; +} + +async function expectSignalActionRejected( + params: Record, + error: RegExp, + cfg: OpenClawConfig, +) { + const handleAction = signalMessageActions.handleAction; + if (!handleAction) { + throw new Error("signal handleAction unavailable"); + } + await expect( + handleAction({ + channel: "signal", + action: "react", + params, + cfg, + accountId: undefined, + }), + ).rejects.toThrow(error); +} + async function expectSlackSendRejected(params: Record, error: RegExp) { const { cfg, actions } = slackHarness(); await expect( @@ -200,37 +259,19 @@ describe("discord message actions", () => { }); it("inherits top-level channel gate when account overrides moderation only", () => { - const cfg = { - channels: { - discord: { - actions: { channels: false }, - accounts: { - vime: { token: "d1", actions: { moderation: true } }, - }, - }, - }, - } as OpenClawConfig; + const cfg = createDiscordModerationOverrideCfg(); const actions = discordMessageActions.listActions?.({ cfg }) ?? []; expect(actions).toContain("timeout"); - expect(actions).not.toContain("channel-create"); + expectChannelCreateAction(actions, false); }); it("allows account to explicitly re-enable top-level disabled channels", () => { - const cfg = { - channels: { - discord: { - actions: { channels: false }, - accounts: { - vime: { token: "d1", actions: { moderation: true, channels: true } }, - }, - }, - }, - } as OpenClawConfig; + const cfg = createDiscordModerationOverrideCfg({ channelsEnabled: true }); const actions = discordMessageActions.listActions?.({ cfg }) ?? []; expect(actions).toContain("timeout"); - expect(actions).toContain("channel-create"); + expectChannelCreateAction(actions, true); }); }); @@ -609,16 +650,7 @@ describe("signalMessageActions", () => { }, { name: "account-level reactions enabled", - cfg: { - channels: { - signal: { - actions: { reactions: false }, - accounts: { - work: { account: "+15550001111", actions: { reactions: true } }, - }, - }, - }, - } as OpenClawConfig, + cfg: createSignalAccountOverrideCfg(), expected: ["send", "react"], }, ] as const; @@ -640,36 +672,18 @@ describe("signalMessageActions", () => { const cfg = { channels: { signal: { account: "+15550001111", actions: { reactions: false } } }, } as OpenClawConfig; - const handleAction = signalMessageActions.handleAction; - if (!handleAction) { - throw new Error("signal handleAction unavailable"); - } - - await expect( - handleAction({ - channel: "signal", - action: "react", - params: { to: "+15550001111", messageId: "123", emoji: "✅" }, - cfg, - accountId: undefined, - }), - ).rejects.toThrow(/actions\.reactions/); + await expectSignalActionRejected( + { to: "+15550001111", messageId: "123", emoji: "✅" }, + /actions\.reactions/, + cfg, + ); }); it("maps reaction targets into signal sendReaction calls", async () => { const cases = [ { name: "uses account-level actions when enabled", - cfg: { - channels: { - signal: { - actions: { reactions: false }, - accounts: { - work: { account: "+15550001111", actions: { reactions: true } }, - }, - }, - }, - } as OpenClawConfig, + cfg: createSignalAccountOverrideCfg(), accountId: "work", params: { to: "+15550001111", messageId: "123", emoji: "👍" }, expectedArgs: ["+15550001111", 123, "👍", { accountId: "work" }], @@ -723,20 +737,11 @@ describe("signalMessageActions", () => { const cfg = { channels: { signal: { account: "+15550001111" } }, } as OpenClawConfig; - const handleAction = signalMessageActions.handleAction; - if (!handleAction) { - throw new Error("signal handleAction unavailable"); - } - - await expect( - handleAction({ - channel: "signal", - action: "react", - params: { to: "signal:group:group-id", messageId: "123", emoji: "✅" }, - cfg, - accountId: undefined, - }), - ).rejects.toThrow(/targetAuthor/); + await expectSignalActionRejected( + { to: "signal:group:group-id", messageId: "123", emoji: "✅" }, + /targetAuthor/, + cfg, + ); }); }); diff --git a/src/channels/plugins/actions/discord.ts b/src/channels/plugins/actions/discord.ts index b174c5050..531301341 100644 --- a/src/channels/plugins/actions/discord.ts +++ b/src/channels/plugins/actions/discord.ts @@ -2,77 +2,79 @@ import type { DiscordActionConfig } from "../../../config/types.discord.js"; import { createDiscordActionGate, listEnabledDiscordAccounts } from "../../../discord/accounts.js"; import type { ChannelMessageActionAdapter, ChannelMessageActionName } from "../types.js"; import { handleDiscordMessageAction } from "./discord/handle-action.js"; +import { createUnionActionGate, listTokenSourcedAccounts } from "./shared.js"; export const discordMessageActions: ChannelMessageActionAdapter = { listActions: ({ cfg }) => { - const accounts = listEnabledDiscordAccounts(cfg).filter( - (account) => account.tokenSource !== "none", - ); + const accounts = listTokenSourcedAccounts(listEnabledDiscordAccounts(cfg)); if (accounts.length === 0) { return []; } // Union of all accounts' action gates (any account enabling an action makes it available) - const gates = accounts.map((account) => - createDiscordActionGate({ cfg, accountId: account.accountId }), + const gate = createUnionActionGate(accounts, (account) => + createDiscordActionGate({ + cfg, + accountId: account.accountId, + }), ); - const gate = (key: keyof DiscordActionConfig, defaultValue = true) => - gates.some((g) => g(key, defaultValue)); + const isEnabled = (key: keyof DiscordActionConfig, defaultValue = true) => + gate(key, defaultValue); const actions = new Set(["send"]); - if (gate("polls")) { + if (isEnabled("polls")) { actions.add("poll"); } - if (gate("reactions")) { + if (isEnabled("reactions")) { actions.add("react"); actions.add("reactions"); } - if (gate("messages")) { + if (isEnabled("messages")) { actions.add("read"); actions.add("edit"); actions.add("delete"); } - if (gate("pins")) { + if (isEnabled("pins")) { actions.add("pin"); actions.add("unpin"); actions.add("list-pins"); } - if (gate("permissions")) { + if (isEnabled("permissions")) { actions.add("permissions"); } - if (gate("threads")) { + if (isEnabled("threads")) { actions.add("thread-create"); actions.add("thread-list"); actions.add("thread-reply"); } - if (gate("search")) { + if (isEnabled("search")) { actions.add("search"); } - if (gate("stickers")) { + if (isEnabled("stickers")) { actions.add("sticker"); } - if (gate("memberInfo")) { + if (isEnabled("memberInfo")) { actions.add("member-info"); } - if (gate("roleInfo")) { + if (isEnabled("roleInfo")) { actions.add("role-info"); } - if (gate("reactions")) { + if (isEnabled("reactions")) { actions.add("emoji-list"); } - if (gate("emojiUploads")) { + if (isEnabled("emojiUploads")) { actions.add("emoji-upload"); } - if (gate("stickerUploads")) { + if (isEnabled("stickerUploads")) { actions.add("sticker-upload"); } - if (gate("roles", false)) { + if (isEnabled("roles", false)) { actions.add("role-add"); actions.add("role-remove"); } - if (gate("channelInfo")) { + if (isEnabled("channelInfo")) { actions.add("channel-info"); actions.add("channel-list"); } - if (gate("channels")) { + if (isEnabled("channels")) { actions.add("channel-create"); actions.add("channel-edit"); actions.add("channel-delete"); @@ -81,19 +83,19 @@ export const discordMessageActions: ChannelMessageActionAdapter = { actions.add("category-edit"); actions.add("category-delete"); } - if (gate("voiceStatus")) { + if (isEnabled("voiceStatus")) { actions.add("voice-status"); } - if (gate("events")) { + if (isEnabled("events")) { actions.add("event-list"); actions.add("event-create"); } - if (gate("moderation", false)) { + if (isEnabled("moderation", false)) { actions.add("timeout"); actions.add("kick"); actions.add("ban"); } - if (gate("presence", false)) { + if (isEnabled("presence", false)) { actions.add("set-presence"); } return Array.from(actions); diff --git a/src/channels/plugins/actions/shared.ts b/src/channels/plugins/actions/shared.ts new file mode 100644 index 000000000..6a9f813d1 --- /dev/null +++ b/src/channels/plugins/actions/shared.ts @@ -0,0 +1,19 @@ +type OptionalDefaultGate = (key: TKey, defaultValue?: boolean) => boolean; + +type TokenSourcedAccount = { + tokenSource?: string | null; +}; + +export function listTokenSourcedAccounts( + accounts: readonly TAccount[], +): TAccount[] { + return accounts.filter((account) => account.tokenSource !== "none"); +} + +export function createUnionActionGate( + accounts: readonly TAccount[], + createGate: (account: TAccount) => OptionalDefaultGate, +): OptionalDefaultGate { + const gates = accounts.map((account) => createGate(account)); + return (key, defaultValue = true) => gates.some((gate) => gate(key, defaultValue)); +} diff --git a/src/channels/plugins/actions/signal.ts b/src/channels/plugins/actions/signal.ts index 7a7ec55bd..db1f06579 100644 --- a/src/channels/plugins/actions/signal.ts +++ b/src/channels/plugins/actions/signal.ts @@ -38,6 +38,34 @@ function resolveSignalReactionTarget(raw: string): { recipient?: string; groupId return { recipient: normalizeSignalReactionRecipient(withoutSignal) }; } +async function mutateSignalReaction(params: { + accountId?: string; + target: { recipient?: string; groupId?: string }; + timestamp: number; + emoji: string; + remove?: boolean; + targetAuthor?: string; + targetAuthorUuid?: string; +}) { + const options = { + accountId: params.accountId, + groupId: params.target.groupId, + targetAuthor: params.targetAuthor, + targetAuthorUuid: params.targetAuthorUuid, + }; + if (params.remove) { + await removeReactionSignal( + params.target.recipient ?? "", + params.timestamp, + params.emoji, + options, + ); + return jsonResult({ ok: true, removed: params.emoji }); + } + await sendReactionSignal(params.target.recipient ?? "", params.timestamp, params.emoji, options); + return jsonResult({ ok: true, added: params.emoji }); +} + export const signalMessageActions: ChannelMessageActionAdapter = { listActions: ({ cfg }) => { const accounts = listEnabledSignalAccounts(cfg); @@ -120,25 +148,29 @@ export const signalMessageActions: ChannelMessageActionAdapter = { if (!emoji) { throw new Error("Emoji required to remove reaction."); } - await removeReactionSignal(target.recipient ?? "", timestamp, emoji, { + return await mutateSignalReaction({ accountId: accountId ?? undefined, - groupId: target.groupId, + target, + timestamp, + emoji, + remove: true, targetAuthor, targetAuthorUuid, }); - return jsonResult({ ok: true, removed: emoji }); } if (!emoji) { throw new Error("Emoji required to add reaction."); } - await sendReactionSignal(target.recipient ?? "", timestamp, emoji, { + return await mutateSignalReaction({ accountId: accountId ?? undefined, - groupId: target.groupId, + target, + timestamp, + emoji, + remove: false, targetAuthor, targetAuthorUuid, }); - return jsonResult({ ok: true, added: emoji }); } throw new Error(`Action ${action} not supported for ${providerId}.`); diff --git a/src/channels/plugins/actions/telegram.ts b/src/channels/plugins/actions/telegram.ts index c0be5c5e4..ebc3c8104 100644 --- a/src/channels/plugins/actions/telegram.ts +++ b/src/channels/plugins/actions/telegram.ts @@ -13,6 +13,7 @@ import { } from "../../../telegram/accounts.js"; import { isTelegramInlineButtonsEnabled } from "../../../telegram/inline-buttons.js"; import type { ChannelMessageActionAdapter, ChannelMessageActionName } from "../types.js"; +import { createUnionActionGate, listTokenSourcedAccounts } from "./shared.js"; const providerId = "telegram"; @@ -41,43 +42,61 @@ function readTelegramSendParams(params: Record) { }; } +function readTelegramChatIdParam(params: Record): string | number { + return ( + readStringOrNumberParam(params, "chatId") ?? + readStringOrNumberParam(params, "channelId") ?? + readStringParam(params, "to", { required: true }) + ); +} + +function readTelegramMessageIdParam(params: Record): number { + const messageId = readNumberParam(params, "messageId", { + required: true, + integer: true, + }); + if (typeof messageId !== "number") { + throw new Error("messageId is required."); + } + return messageId; +} + export const telegramMessageActions: ChannelMessageActionAdapter = { listActions: ({ cfg }) => { - const accounts = listEnabledTelegramAccounts(cfg).filter( - (account) => account.tokenSource !== "none", - ); + const accounts = listTokenSourcedAccounts(listEnabledTelegramAccounts(cfg)); if (accounts.length === 0) { return []; } // Union of all accounts' action gates (any account enabling an action makes it available) - const gates = accounts.map((account) => - createTelegramActionGate({ cfg, accountId: account.accountId }), + const gate = createUnionActionGate(accounts, (account) => + createTelegramActionGate({ + cfg, + accountId: account.accountId, + }), ); - const gate = (key: keyof TelegramActionConfig, defaultValue = true) => - gates.some((g) => g(key, defaultValue)); + const isEnabled = (key: keyof TelegramActionConfig, defaultValue = true) => + gate(key, defaultValue); const actions = new Set(["send"]); - if (gate("reactions")) { + if (isEnabled("reactions")) { actions.add("react"); } - if (gate("deleteMessage")) { + if (isEnabled("deleteMessage")) { actions.add("delete"); } - if (gate("editMessage")) { + if (isEnabled("editMessage")) { actions.add("edit"); } - if (gate("sticker", false)) { + if (isEnabled("sticker", false)) { actions.add("sticker"); actions.add("sticker-search"); } - if (gate("createForumTopic")) { + if (isEnabled("createForumTopic")) { actions.add("topic-create"); } return Array.from(actions); }, supportsButtons: ({ cfg }) => { - const accounts = listEnabledTelegramAccounts(cfg).filter( - (account) => account.tokenSource !== "none", - ); + const accounts = listTokenSourcedAccounts(listEnabledTelegramAccounts(cfg)); if (accounts.length === 0) { return false; } @@ -110,10 +129,7 @@ export const telegramMessageActions: ChannelMessageActionAdapter = { return await handleTelegramAction( { action: "react", - chatId: - readStringOrNumberParam(params, "chatId") ?? - readStringOrNumberParam(params, "channelId") ?? - readStringParam(params, "to", { required: true }), + chatId: readTelegramChatIdParam(params), messageId, emoji, remove, @@ -124,14 +140,8 @@ export const telegramMessageActions: ChannelMessageActionAdapter = { } if (action === "delete") { - const chatId = - readStringOrNumberParam(params, "chatId") ?? - readStringOrNumberParam(params, "channelId") ?? - readStringParam(params, "to", { required: true }); - const messageId = readNumberParam(params, "messageId", { - required: true, - integer: true, - }); + const chatId = readTelegramChatIdParam(params); + const messageId = readTelegramMessageIdParam(params); return await handleTelegramAction( { action: "deleteMessage", @@ -144,14 +154,8 @@ export const telegramMessageActions: ChannelMessageActionAdapter = { } if (action === "edit") { - const chatId = - readStringOrNumberParam(params, "chatId") ?? - readStringOrNumberParam(params, "channelId") ?? - readStringParam(params, "to", { required: true }); - const messageId = readNumberParam(params, "messageId", { - required: true, - integer: true, - }); + const chatId = readTelegramChatIdParam(params); + const messageId = readTelegramMessageIdParam(params); const message = readStringParam(params, "message", { required: true, allowEmpty: false }); const buttons = params.buttons; return await handleTelegramAction( @@ -203,10 +207,7 @@ export const telegramMessageActions: ChannelMessageActionAdapter = { } if (action === "topic-create") { - const chatId = - readStringOrNumberParam(params, "chatId") ?? - readStringOrNumberParam(params, "channelId") ?? - readStringParam(params, "to", { required: true }); + const chatId = readTelegramChatIdParam(params); const name = readStringParam(params, "name", { required: true }); const iconColor = readNumberParam(params, "iconColor", { integer: true }); const iconCustomEmojiId = readStringParam(params, "iconCustomEmojiId"); diff --git a/src/channels/plugins/directory-config.ts b/src/channels/plugins/directory-config.ts index eaec8e7b5..66620e442 100644 --- a/src/channels/plugins/directory-config.ts +++ b/src/channels/plugins/directory-config.ts @@ -26,14 +26,33 @@ function addAllowFromAndDmsIds( } ids.add(raw); } - for (const id of Object.keys(dms ?? {})) { - const trimmed = id.trim(); - if (trimmed) { - ids.add(trimmed); - } + addTrimmedEntries(ids, Object.keys(dms ?? {})); +} + +function addTrimmedId(ids: Set, value: unknown) { + const trimmed = String(value).trim(); + if (trimmed) { + ids.add(trimmed); } } +function addTrimmedEntries(ids: Set, values: Iterable) { + for (const value of values) { + addTrimmedId(ids, value); + } +} + +function normalizeTrimmedSet( + ids: Set, + normalize: (raw: string) => string | null, +): string[] { + return Array.from(ids) + .map((raw) => raw.trim()) + .filter(Boolean) + .map((raw) => normalize(raw)) + .filter((id): id is string => Boolean(id)); +} + function resolveDirectoryQuery(query?: string | null): string { return query?.trim().toLowerCase() || ""; } @@ -61,28 +80,18 @@ export async function listSlackDirectoryPeersFromConfig( addAllowFromAndDmsIds(ids, account.config.allowFrom ?? account.dm?.allowFrom, account.config.dms); for (const channel of Object.values(account.config.channels ?? {})) { - for (const user of channel.users ?? []) { - const raw = String(user).trim(); - if (raw) { - ids.add(raw); - } - } + addTrimmedEntries(ids, channel.users ?? []); } - const normalizedIds = Array.from(ids) - .map((raw) => raw.trim()) - .filter(Boolean) - .map((raw) => { - const mention = raw.match(/^<@([A-Z0-9]+)>$/i); - const normalizedUserId = (mention?.[1] ?? raw).replace(/^(slack|user):/i, "").trim(); - if (!normalizedUserId) { - return null; - } - const target = `user:${normalizedUserId}`; - return normalizeSlackMessagingTarget(target) ?? target.toLowerCase(); - }) - .filter((id): id is string => Boolean(id)) - .filter((id) => id.startsWith("user:")); + const normalizedIds = normalizeTrimmedSet(ids, (raw) => { + const mention = raw.match(/^<@([A-Z0-9]+)>$/i); + const normalizedUserId = (mention?.[1] ?? raw).replace(/^(slack|user):/i, "").trim(); + if (!normalizedUserId) { + return null; + } + const target = `user:${normalizedUserId}`; + return normalizeSlackMessagingTarget(target) ?? target.toLowerCase(); + }).filter((id) => id.startsWith("user:")); return toDirectoryEntries("user", applyDirectoryQueryAndLimit(normalizedIds, params)); } @@ -110,34 +119,20 @@ export async function listDiscordDirectoryPeersFromConfig( account.config.dms, ); for (const guild of Object.values(account.config.guilds ?? {})) { - for (const entry of guild.users ?? []) { - const raw = String(entry).trim(); - if (raw) { - ids.add(raw); - } - } + addTrimmedEntries(ids, guild.users ?? []); for (const channel of Object.values(guild.channels ?? {})) { - for (const user of channel.users ?? []) { - const raw = String(user).trim(); - if (raw) { - ids.add(raw); - } - } + addTrimmedEntries(ids, channel.users ?? []); } } - const normalizedIds = Array.from(ids) - .map((raw) => raw.trim()) - .filter(Boolean) - .map((raw) => { - const mention = raw.match(/^<@!?(\d+)>$/); - const cleaned = (mention?.[1] ?? raw).replace(/^(discord|user):/i, "").trim(); - if (!/^\d+$/.test(cleaned)) { - return null; - } - return `user:${cleaned}`; - }) - .filter((id): id is string => Boolean(id)); + const normalizedIds = normalizeTrimmedSet(ids, (raw) => { + const mention = raw.match(/^<@!?(\d+)>$/); + const cleaned = (mention?.[1] ?? raw).replace(/^(discord|user):/i, "").trim(); + if (!/^\d+$/.test(cleaned)) { + return null; + } + return `user:${cleaned}`; + }); return toDirectoryEntries("user", applyDirectoryQueryAndLimit(normalizedIds, params)); } @@ -147,26 +142,17 @@ export async function listDiscordDirectoryGroupsFromConfig( const account = resolveDiscordAccount({ cfg: params.cfg, accountId: params.accountId }); const ids = new Set(); for (const guild of Object.values(account.config.guilds ?? {})) { - for (const channelId of Object.keys(guild.channels ?? {})) { - const trimmed = channelId.trim(); - if (trimmed) { - ids.add(trimmed); - } - } + addTrimmedEntries(ids, Object.keys(guild.channels ?? {})); } - const normalizedIds = Array.from(ids) - .map((raw) => raw.trim()) - .filter(Boolean) - .map((raw) => { - const mention = raw.match(/^<#(\d+)>$/); - const cleaned = (mention?.[1] ?? raw).replace(/^(discord|channel|group):/i, "").trim(); - if (!/^\d+$/.test(cleaned)) { - return null; - } - return `channel:${cleaned}`; - }) - .filter((id): id is string => Boolean(id)); + const normalizedIds = normalizeTrimmedSet(ids, (raw) => { + const mention = raw.match(/^<#(\d+)>$/); + const cleaned = (mention?.[1] ?? raw).replace(/^(discord|channel|group):/i, "").trim(); + if (!/^\d+$/.test(cleaned)) { + return null; + } + return `channel:${cleaned}`; + }); return toDirectoryEntries("group", applyDirectoryQueryAndLimit(normalizedIds, params)); } diff --git a/src/channels/plugins/group-mentions.test.ts b/src/channels/plugins/group-mentions.test.ts index cc0c3668a..0d6a6dca2 100644 --- a/src/channels/plugins/group-mentions.test.ts +++ b/src/channels/plugins/group-mentions.test.ts @@ -1,5 +1,14 @@ import { describe, expect, it } from "vitest"; -import { resolveSlackGroupRequireMention, resolveSlackGroupToolPolicy } from "./group-mentions.js"; +import { + resolveBlueBubblesGroupRequireMention, + resolveBlueBubblesGroupToolPolicy, + resolveDiscordGroupRequireMention, + resolveDiscordGroupToolPolicy, + resolveSlackGroupRequireMention, + resolveSlackGroupToolPolicy, + resolveTelegramGroupRequireMention, + resolveTelegramGroupToolPolicy, +} from "./group-mentions.js"; const cfg = { channels: { @@ -53,3 +62,149 @@ describe("group mentions (slack)", () => { expect(wildcardTools).toEqual({ deny: ["exec"] }); }); }); + +describe("group mentions (telegram)", () => { + it("resolves topic-level requireMention and chat-level tools for topic ids", () => { + const telegramCfg = { + channels: { + telegram: { + botToken: "telegram-test", + groups: { + "-1001": { + requireMention: true, + tools: { allow: ["message.send"] }, + topics: { + "77": { + requireMention: false, + }, + }, + }, + "*": { + requireMention: true, + }, + }, + }, + }, + // oxlint-disable-next-line typescript/no-explicit-any + } as any; + expect( + resolveTelegramGroupRequireMention({ cfg: telegramCfg, groupId: "-1001:topic:77" }), + ).toBe(false); + expect(resolveTelegramGroupToolPolicy({ cfg: telegramCfg, groupId: "-1001:topic:77" })).toEqual( + { + allow: ["message.send"], + }, + ); + }); +}); + +describe("group mentions (discord)", () => { + it("prefers channel policy, then guild policy, with sender-specific overrides", () => { + const discordCfg = { + channels: { + discord: { + token: "discord-test", + guilds: { + guild1: { + requireMention: false, + tools: { allow: ["message.guild"] }, + toolsBySender: { + "user:guild-admin": { allow: ["sessions.list"] }, + }, + channels: { + "123": { + requireMention: true, + tools: { allow: ["message.channel"] }, + toolsBySender: { + "user:channel-admin": { deny: ["exec"] }, + }, + }, + }, + }, + }, + }, + }, + // oxlint-disable-next-line typescript/no-explicit-any + } as any; + + expect( + resolveDiscordGroupRequireMention({ cfg: discordCfg, groupSpace: "guild1", groupId: "123" }), + ).toBe(true); + expect( + resolveDiscordGroupRequireMention({ + cfg: discordCfg, + groupSpace: "guild1", + groupId: "missing", + }), + ).toBe(false); + expect( + resolveDiscordGroupToolPolicy({ + cfg: discordCfg, + groupSpace: "guild1", + groupId: "123", + senderId: "user:channel-admin", + }), + ).toEqual({ deny: ["exec"] }); + expect( + resolveDiscordGroupToolPolicy({ + cfg: discordCfg, + groupSpace: "guild1", + groupId: "123", + senderId: "user:someone", + }), + ).toEqual({ allow: ["message.channel"] }); + expect( + resolveDiscordGroupToolPolicy({ + cfg: discordCfg, + groupSpace: "guild1", + groupId: "missing", + senderId: "user:guild-admin", + }), + ).toEqual({ allow: ["sessions.list"] }); + expect( + resolveDiscordGroupToolPolicy({ + cfg: discordCfg, + groupSpace: "guild1", + groupId: "missing", + senderId: "user:someone", + }), + ).toEqual({ allow: ["message.guild"] }); + }); +}); + +describe("group mentions (bluebubbles)", () => { + it("uses generic channel group policy helpers", () => { + const blueBubblesCfg = { + channels: { + bluebubbles: { + groups: { + "chat:primary": { + requireMention: false, + tools: { deny: ["exec"] }, + }, + "*": { + requireMention: true, + tools: { allow: ["message.send"] }, + }, + }, + }, + }, + // oxlint-disable-next-line typescript/no-explicit-any + } as any; + + expect( + resolveBlueBubblesGroupRequireMention({ cfg: blueBubblesCfg, groupId: "chat:primary" }), + ).toBe(false); + expect( + resolveBlueBubblesGroupRequireMention({ cfg: blueBubblesCfg, groupId: "chat:other" }), + ).toBe(true); + expect( + resolveBlueBubblesGroupToolPolicy({ cfg: blueBubblesCfg, groupId: "chat:primary" }), + ).toEqual({ deny: ["exec"] }); + expect( + resolveBlueBubblesGroupToolPolicy({ cfg: blueBubblesCfg, groupId: "chat:other" }), + ).toEqual({ + allow: ["message.send"], + }); + }); +}); diff --git a/src/channels/plugins/group-mentions.ts b/src/channels/plugins/group-mentions.ts index 719409701..0988e2e66 100644 --- a/src/channels/plugins/group-mentions.ts +++ b/src/channels/plugins/group-mentions.ts @@ -11,18 +11,9 @@ import type { } from "../../config/types.tools.js"; import { normalizeAtHashSlug, normalizeHyphenSlug } from "../../shared/string-normalization.js"; import { resolveSlackAccount } from "../../slack/accounts.js"; +import type { ChannelGroupContext } from "./types.js"; -type GroupMentionParams = { - cfg: OpenClawConfig; - groupId?: string | null; - groupChannel?: string | null; - groupSpace?: string | null; - accountId?: string | null; - senderId?: string | null; - senderName?: string | null; - senderUsername?: string | null; - senderE164?: string | null; -}; +type GroupMentionParams = ChannelGroupContext; function normalizeDiscordSlug(value?: string | null) { return normalizeAtHashSlug(value); @@ -124,6 +115,18 @@ type SlackChannelPolicyEntry = { toolsBySender?: GroupToolPolicyBySenderConfig; }; +type SenderScopedToolsEntry = { + tools?: GroupToolPolicyConfig; + toolsBySender?: GroupToolPolicyBySenderConfig; +}; + +type ChannelGroupPolicyChannel = + | "telegram" + | "whatsapp" + | "imessage" + | "googlechat" + | "bluebubbles"; + function resolveSlackChannelPolicyEntry( params: GroupMentionParams, ): SlackChannelPolicyEntry | undefined { @@ -153,6 +156,69 @@ function resolveSlackChannelPolicyEntry( return channels["*"]; } +function resolveChannelRequireMention( + params: GroupMentionParams, + channel: ChannelGroupPolicyChannel, + groupId: string | null | undefined = params.groupId, +): boolean { + return resolveChannelGroupRequireMention({ + cfg: params.cfg, + channel, + groupId, + accountId: params.accountId, + }); +} + +function resolveChannelToolPolicyForSender( + params: GroupMentionParams, + channel: ChannelGroupPolicyChannel, + groupId: string | null | undefined = params.groupId, +): GroupToolPolicyConfig | undefined { + return resolveChannelGroupToolsPolicy({ + cfg: params.cfg, + channel, + groupId, + accountId: params.accountId, + senderId: params.senderId, + senderName: params.senderName, + senderUsername: params.senderUsername, + senderE164: params.senderE164, + }); +} + +function resolveSenderToolsEntry( + entry: SenderScopedToolsEntry | undefined | null, + params: GroupMentionParams, +): GroupToolPolicyConfig | undefined { + if (!entry) { + return undefined; + } + const senderPolicy = resolveToolsBySender({ + toolsBySender: entry.toolsBySender, + senderId: params.senderId, + senderName: params.senderName, + senderUsername: params.senderUsername, + senderE164: params.senderE164, + }); + if (senderPolicy) { + return senderPolicy; + } + return entry.tools; +} + +function resolveDiscordPolicyContext(params: GroupMentionParams) { + const guildEntry = resolveDiscordGuildEntry( + params.cfg.channels?.discord?.guilds, + params.groupSpace, + ); + const channelEntries = guildEntry?.channels; + const channelEntry = + channelEntries && Object.keys(channelEntries).length > 0 + ? resolveDiscordChannelEntry(channelEntries, params) + : undefined; + return { guildEntry, channelEntry }; +} + export function resolveTelegramGroupRequireMention( params: GroupMentionParams, ): boolean | undefined { @@ -174,63 +240,32 @@ export function resolveTelegramGroupRequireMention( } export function resolveWhatsAppGroupRequireMention(params: GroupMentionParams): boolean { - return resolveChannelGroupRequireMention({ - cfg: params.cfg, - channel: "whatsapp", - groupId: params.groupId, - accountId: params.accountId, - }); + return resolveChannelRequireMention(params, "whatsapp"); } export function resolveIMessageGroupRequireMention(params: GroupMentionParams): boolean { - return resolveChannelGroupRequireMention({ - cfg: params.cfg, - channel: "imessage", - groupId: params.groupId, - accountId: params.accountId, - }); + return resolveChannelRequireMention(params, "imessage"); } export function resolveDiscordGroupRequireMention(params: GroupMentionParams): boolean { - const guildEntry = resolveDiscordGuildEntry( - params.cfg.channels?.discord?.guilds, - params.groupSpace, - ); - const channelEntries = guildEntry?.channels; - if (channelEntries && Object.keys(channelEntries).length > 0) { - const entry = resolveDiscordChannelEntry(channelEntries, params); - if (entry && typeof entry.requireMention === "boolean") { - return entry.requireMention; - } + const context = resolveDiscordPolicyContext(params); + if (typeof context.channelEntry?.requireMention === "boolean") { + return context.channelEntry.requireMention; } - if (typeof guildEntry?.requireMention === "boolean") { - return guildEntry.requireMention; + if (typeof context.guildEntry?.requireMention === "boolean") { + return context.guildEntry.requireMention; } return true; } export function resolveGoogleChatGroupRequireMention(params: GroupMentionParams): boolean { - return resolveChannelGroupRequireMention({ - cfg: params.cfg, - channel: "googlechat", - groupId: params.groupId, - accountId: params.accountId, - }); + return resolveChannelRequireMention(params, "googlechat"); } export function resolveGoogleChatGroupToolPolicy( params: GroupMentionParams, ): GroupToolPolicyConfig | undefined { - return resolveChannelGroupToolsPolicy({ - cfg: params.cfg, - channel: "googlechat", - groupId: params.groupId, - accountId: params.accountId, - senderId: params.senderId, - senderName: params.senderName, - senderUsername: params.senderUsername, - senderE164: params.senderE164, - }); + return resolveChannelToolPolicyForSender(params, "googlechat"); } export function resolveSlackGroupRequireMention(params: GroupMentionParams): boolean { @@ -242,134 +277,48 @@ export function resolveSlackGroupRequireMention(params: GroupMentionParams): boo } export function resolveBlueBubblesGroupRequireMention(params: GroupMentionParams): boolean { - return resolveChannelGroupRequireMention({ - cfg: params.cfg, - channel: "bluebubbles", - groupId: params.groupId, - accountId: params.accountId, - }); + return resolveChannelRequireMention(params, "bluebubbles"); } export function resolveTelegramGroupToolPolicy( params: GroupMentionParams, ): GroupToolPolicyConfig | undefined { const { chatId } = parseTelegramGroupId(params.groupId); - return resolveChannelGroupToolsPolicy({ - cfg: params.cfg, - channel: "telegram", - groupId: chatId ?? params.groupId, - accountId: params.accountId, - senderId: params.senderId, - senderName: params.senderName, - senderUsername: params.senderUsername, - senderE164: params.senderE164, - }); + return resolveChannelToolPolicyForSender(params, "telegram", chatId ?? params.groupId); } export function resolveWhatsAppGroupToolPolicy( params: GroupMentionParams, ): GroupToolPolicyConfig | undefined { - return resolveChannelGroupToolsPolicy({ - cfg: params.cfg, - channel: "whatsapp", - groupId: params.groupId, - accountId: params.accountId, - senderId: params.senderId, - senderName: params.senderName, - senderUsername: params.senderUsername, - senderE164: params.senderE164, - }); + return resolveChannelToolPolicyForSender(params, "whatsapp"); } export function resolveIMessageGroupToolPolicy( params: GroupMentionParams, ): GroupToolPolicyConfig | undefined { - return resolveChannelGroupToolsPolicy({ - cfg: params.cfg, - channel: "imessage", - groupId: params.groupId, - accountId: params.accountId, - senderId: params.senderId, - senderName: params.senderName, - senderUsername: params.senderUsername, - senderE164: params.senderE164, - }); + return resolveChannelToolPolicyForSender(params, "imessage"); } export function resolveDiscordGroupToolPolicy( params: GroupMentionParams, ): GroupToolPolicyConfig | undefined { - const guildEntry = resolveDiscordGuildEntry( - params.cfg.channels?.discord?.guilds, - params.groupSpace, - ); - const channelEntries = guildEntry?.channels; - if (channelEntries && Object.keys(channelEntries).length > 0) { - const entry = resolveDiscordChannelEntry(channelEntries, params); - const senderPolicy = resolveToolsBySender({ - toolsBySender: entry?.toolsBySender, - senderId: params.senderId, - senderName: params.senderName, - senderUsername: params.senderUsername, - senderE164: params.senderE164, - }); - if (senderPolicy) { - return senderPolicy; - } - if (entry?.tools) { - return entry.tools; - } + const context = resolveDiscordPolicyContext(params); + const channelPolicy = resolveSenderToolsEntry(context.channelEntry, params); + if (channelPolicy) { + return channelPolicy; } - const guildSenderPolicy = resolveToolsBySender({ - toolsBySender: guildEntry?.toolsBySender, - senderId: params.senderId, - senderName: params.senderName, - senderUsername: params.senderUsername, - senderE164: params.senderE164, - }); - if (guildSenderPolicy) { - return guildSenderPolicy; - } - if (guildEntry?.tools) { - return guildEntry.tools; - } - return undefined; + return resolveSenderToolsEntry(context.guildEntry, params); } export function resolveSlackGroupToolPolicy( params: GroupMentionParams, ): GroupToolPolicyConfig | undefined { const resolved = resolveSlackChannelPolicyEntry(params); - if (!resolved) { - return undefined; - } - const senderPolicy = resolveToolsBySender({ - toolsBySender: resolved?.toolsBySender, - senderId: params.senderId, - senderName: params.senderName, - senderUsername: params.senderUsername, - senderE164: params.senderE164, - }); - if (senderPolicy) { - return senderPolicy; - } - if (resolved?.tools) { - return resolved.tools; - } - return undefined; + return resolveSenderToolsEntry(resolved, params); } export function resolveBlueBubblesGroupToolPolicy( params: GroupMentionParams, ): GroupToolPolicyConfig | undefined { - return resolveChannelGroupToolsPolicy({ - cfg: params.cfg, - channel: "bluebubbles", - groupId: params.groupId, - accountId: params.accountId, - senderId: params.senderId, - senderName: params.senderName, - senderUsername: params.senderUsername, - senderE164: params.senderE164, - }); + return resolveChannelToolPolicyForSender(params, "bluebubbles"); } diff --git a/src/channels/plugins/load.ts b/src/channels/plugins/load.ts index e339b54f3..70ebe45a8 100644 --- a/src/channels/plugins/load.ts +++ b/src/channels/plugins/load.ts @@ -1,29 +1,8 @@ -import type { PluginRegistry } from "../../plugins/registry.js"; -import { getActivePluginRegistry } from "../../plugins/runtime.js"; +import { createChannelRegistryLoader } from "./registry-loader.js"; import type { ChannelId, ChannelPlugin } from "./types.js"; -const cache = new Map(); -let lastRegistry: PluginRegistry | null = null; - -function ensureCacheForRegistry(registry: PluginRegistry | null) { - if (registry === lastRegistry) { - return; - } - cache.clear(); - lastRegistry = registry; -} +const loadPluginFromRegistry = createChannelRegistryLoader((entry) => entry.plugin); export async function loadChannelPlugin(id: ChannelId): Promise { - const registry = getActivePluginRegistry(); - ensureCacheForRegistry(registry); - const cached = cache.get(id); - if (cached) { - return cached; - } - const pluginEntry = registry?.channels.find((entry) => entry.plugin.id === id); - if (pluginEntry) { - cache.set(id, pluginEntry.plugin); - return pluginEntry.plugin; - } - return undefined; + return loadPluginFromRegistry(id); } diff --git a/src/channels/plugins/message-actions.security.test.ts b/src/channels/plugins/message-actions.security.test.ts index 0ca6ec36d..1dbd19de3 100644 --- a/src/channels/plugins/message-actions.security.test.ts +++ b/src/channels/plugins/message-actions.security.test.ts @@ -2,7 +2,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { jsonResult } from "../../agents/tools/common.js"; import type { OpenClawConfig } from "../../config/config.js"; import { setActivePluginRegistry } from "../../plugins/runtime.js"; -import { createTestRegistry } from "../../test-utils/channel-plugins.js"; +import { + createChannelTestPluginBase, + createTestRegistry, +} from "../../test-utils/channel-plugins.js"; import { dispatchChannelMessageAction } from "./message-actions.js"; import type { ChannelPlugin } from "./types.js"; @@ -11,19 +14,14 @@ const handleAction = vi.fn(async () => jsonResult({ ok: true })); const emptyRegistry = createTestRegistry([]); const discordPlugin: ChannelPlugin = { - id: "discord", - meta: { + ...createChannelTestPluginBase({ id: "discord", label: "Discord", - selectionLabel: "Discord", - docsPath: "/channels/discord", - blurb: "Discord test plugin.", - }, - capabilities: { chatTypes: ["direct", "group"] }, - config: { - listAccountIds: () => ["default"], - resolveAccount: () => ({}), - }, + capabilities: { chatTypes: ["direct", "group"] }, + config: { + listAccountIds: () => ["default"], + }, + }), actions: { listActions: () => ["kick"], supportsAction: ({ action }) => action === "kick", diff --git a/src/channels/plugins/message-actions.test.ts b/src/channels/plugins/message-actions.test.ts new file mode 100644 index 000000000..6a292463b --- /dev/null +++ b/src/channels/plugins/message-actions.test.ts @@ -0,0 +1,87 @@ +import { afterEach, describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../../config/config.js"; +import { setActivePluginRegistry } from "../../plugins/runtime.js"; +import { + createChannelTestPluginBase, + createTestRegistry, +} from "../../test-utils/channel-plugins.js"; +import { + supportsChannelMessageButtons, + supportsChannelMessageButtonsForChannel, + supportsChannelMessageCards, + supportsChannelMessageCardsForChannel, +} from "./message-actions.js"; +import type { ChannelPlugin } from "./types.js"; + +const emptyRegistry = createTestRegistry([]); + +function createMessageActionsPlugin(params: { + id: "discord" | "telegram"; + supportsButtons: boolean; + supportsCards: boolean; +}): ChannelPlugin { + return { + ...createChannelTestPluginBase({ + id: params.id, + label: params.id === "discord" ? "Discord" : "Telegram", + capabilities: { chatTypes: ["direct", "group"] }, + config: { + listAccountIds: () => ["default"], + }, + }), + actions: { + listActions: () => ["send"], + supportsButtons: () => params.supportsButtons, + supportsCards: () => params.supportsCards, + }, + }; +} + +const buttonsPlugin = createMessageActionsPlugin({ + id: "discord", + supportsButtons: true, + supportsCards: false, +}); + +const cardsPlugin = createMessageActionsPlugin({ + id: "telegram", + supportsButtons: false, + supportsCards: true, +}); + +function activateMessageActionTestRegistry() { + setActivePluginRegistry( + createTestRegistry([ + { pluginId: "discord", source: "test", plugin: buttonsPlugin }, + { pluginId: "telegram", source: "test", plugin: cardsPlugin }, + ]), + ); +} + +describe("message action capability checks", () => { + afterEach(() => { + setActivePluginRegistry(emptyRegistry); + }); + + it("aggregates buttons/card support across plugins", () => { + activateMessageActionTestRegistry(); + + expect(supportsChannelMessageButtons({} as OpenClawConfig)).toBe(true); + expect(supportsChannelMessageCards({} as OpenClawConfig)).toBe(true); + }); + + it("checks per-channel capabilities", () => { + activateMessageActionTestRegistry(); + + expect( + supportsChannelMessageButtonsForChannel({ cfg: {} as OpenClawConfig, channel: "discord" }), + ).toBe(true); + expect( + supportsChannelMessageButtonsForChannel({ cfg: {} as OpenClawConfig, channel: "telegram" }), + ).toBe(false); + expect( + supportsChannelMessageCardsForChannel({ cfg: {} as OpenClawConfig, channel: "telegram" }), + ).toBe(true); + expect(supportsChannelMessageCardsForChannel({ cfg: {} as OpenClawConfig })).toBe(false); + }); +}); diff --git a/src/channels/plugins/message-actions.ts b/src/channels/plugins/message-actions.ts index da242fa43..a7b8e6aa5 100644 --- a/src/channels/plugins/message-actions.ts +++ b/src/channels/plugins/message-actions.ts @@ -9,6 +9,8 @@ const trustedRequesterRequiredByChannel: Readonly< discord: new Set(["timeout", "kick", "ban"]), }; +type ChannelActions = NonNullable>["actions"]>; + function requiresTrustedRequesterSender(ctx: ChannelMessageActionContext): boolean { const actions = trustedRequesterRequiredByChannel[ctx.channel]; return Boolean(actions?.has(ctx.action) && ctx.toolContext); @@ -29,43 +31,57 @@ export function listChannelMessageActions(cfg: OpenClawConfig): ChannelMessageAc } export function supportsChannelMessageButtons(cfg: OpenClawConfig): boolean { - for (const plugin of listChannelPlugins()) { - if (plugin.actions?.supportsButtons?.({ cfg })) { - return true; - } - } - return false; + return supportsMessageFeature(cfg, (actions) => actions?.supportsButtons?.({ cfg }) === true); } export function supportsChannelMessageButtonsForChannel(params: { cfg: OpenClawConfig; channel?: string; }): boolean { - if (!params.channel) { - return false; - } - const plugin = getChannelPlugin(params.channel as Parameters[0]); - return plugin?.actions?.supportsButtons?.({ cfg: params.cfg }) === true; + return supportsMessageFeatureForChannel( + params, + (actions) => actions.supportsButtons?.(params) === true, + ); } export function supportsChannelMessageCards(cfg: OpenClawConfig): boolean { - for (const plugin of listChannelPlugins()) { - if (plugin.actions?.supportsCards?.({ cfg })) { - return true; - } - } - return false; + return supportsMessageFeature(cfg, (actions) => actions?.supportsCards?.({ cfg }) === true); } export function supportsChannelMessageCardsForChannel(params: { cfg: OpenClawConfig; channel?: string; }): boolean { + return supportsMessageFeatureForChannel( + params, + (actions) => actions.supportsCards?.(params) === true, + ); +} + +function supportsMessageFeature( + cfg: OpenClawConfig, + check: (actions: ChannelActions) => boolean, +): boolean { + for (const plugin of listChannelPlugins()) { + if (plugin.actions && check(plugin.actions)) { + return true; + } + } + return false; +} + +function supportsMessageFeatureForChannel( + params: { + cfg: OpenClawConfig; + channel?: string; + }, + check: (actions: ChannelActions) => boolean, +): boolean { if (!params.channel) { return false; } const plugin = getChannelPlugin(params.channel as Parameters[0]); - return plugin?.actions?.supportsCards?.({ cfg: params.cfg }) === true; + return plugin?.actions ? check(plugin.actions) : false; } export async function dispatchChannelMessageAction( diff --git a/src/channels/plugins/normalize/imessage.ts b/src/channels/plugins/normalize/imessage.ts index aa5b542de..94cb58338 100644 --- a/src/channels/plugins/normalize/imessage.ts +++ b/src/channels/plugins/normalize/imessage.ts @@ -1,4 +1,5 @@ import { normalizeIMessageHandle } from "../../../imessage/targets.js"; +import { looksLikeHandleOrPhoneTarget, trimMessagingTarget } from "./shared.js"; // Service prefixes that indicate explicit delivery method; must be preserved during normalization const SERVICE_PREFIXES = ["imessage:", "sms:", "auto:"] as const; @@ -6,7 +7,7 @@ const CHAT_TARGET_PREFIX_RE = /^(chat_id:|chatid:|chat:|chat_guid:|chatguid:|guid:|chat_identifier:|chatidentifier:|chatident:)/i; export function normalizeIMessageMessagingTarget(raw: string): string | undefined { - const trimmed = raw.trim(); + const trimmed = trimMessagingTarget(raw); if (!trimmed) { return undefined; } @@ -32,18 +33,15 @@ export function normalizeIMessageMessagingTarget(raw: string): string | undefine } export function looksLikeIMessageTargetId(raw: string): boolean { - const trimmed = raw.trim(); + const trimmed = trimMessagingTarget(raw); if (!trimmed) { return false; } - if (/^(imessage:|sms:|auto:)/i.test(trimmed)) { - return true; - } if (CHAT_TARGET_PREFIX_RE.test(trimmed)) { return true; } - if (trimmed.includes("@")) { - return true; - } - return /^\+?\d{3,}$/.test(trimmed); + return looksLikeHandleOrPhoneTarget({ + raw: trimmed, + prefixPattern: /^(imessage:|sms:|auto:)/i, + }); } diff --git a/src/channels/plugins/normalize/shared.ts b/src/channels/plugins/normalize/shared.ts new file mode 100644 index 000000000..270235b12 --- /dev/null +++ b/src/channels/plugins/normalize/shared.ts @@ -0,0 +1,22 @@ +export function trimMessagingTarget(raw: string): string | undefined { + const trimmed = raw.trim(); + return trimmed || undefined; +} + +export function looksLikeHandleOrPhoneTarget(params: { + raw: string; + prefixPattern: RegExp; + phonePattern?: RegExp; +}): boolean { + const trimmed = params.raw.trim(); + if (!trimmed) { + return false; + } + if (params.prefixPattern.test(trimmed)) { + return true; + } + if (trimmed.includes("@")) { + return true; + } + return (params.phonePattern ?? /^\+?\d{3,}$/).test(trimmed); +} diff --git a/src/channels/plugins/normalize/targets.test.ts b/src/channels/plugins/normalize/targets.test.ts new file mode 100644 index 000000000..cf30f51af --- /dev/null +++ b/src/channels/plugins/normalize/targets.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; +import { looksLikeIMessageTargetId, normalizeIMessageMessagingTarget } from "./imessage.js"; +import { looksLikeWhatsAppTargetId, normalizeWhatsAppMessagingTarget } from "./whatsapp.js"; + +describe("normalize target helpers", () => { + describe("iMessage", () => { + it("normalizes blank inputs to undefined", () => { + expect(normalizeIMessageMessagingTarget(" ")).toBeUndefined(); + }); + + it("detects common iMessage target forms", () => { + expect(looksLikeIMessageTargetId("sms:+15555550123")).toBe(true); + expect(looksLikeIMessageTargetId("chat_id:123")).toBe(true); + expect(looksLikeIMessageTargetId("user@example.com")).toBe(true); + expect(looksLikeIMessageTargetId("+15555550123")).toBe(true); + expect(looksLikeIMessageTargetId("")).toBe(false); + }); + }); + + describe("WhatsApp", () => { + it("normalizes blank inputs to undefined", () => { + expect(normalizeWhatsAppMessagingTarget(" ")).toBeUndefined(); + }); + + it("detects common WhatsApp target forms", () => { + expect(looksLikeWhatsAppTargetId("whatsapp:+15555550123")).toBe(true); + expect(looksLikeWhatsAppTargetId("15555550123@c.us")).toBe(true); + expect(looksLikeWhatsAppTargetId("+15555550123")).toBe(true); + expect(looksLikeWhatsAppTargetId("")).toBe(false); + }); + }); +}); diff --git a/src/channels/plugins/normalize/whatsapp.ts b/src/channels/plugins/normalize/whatsapp.ts index af7f5fffa..3504766cc 100644 --- a/src/channels/plugins/normalize/whatsapp.ts +++ b/src/channels/plugins/normalize/whatsapp.ts @@ -1,7 +1,8 @@ import { normalizeWhatsAppTarget } from "../../../whatsapp/normalize.js"; +import { looksLikeHandleOrPhoneTarget, trimMessagingTarget } from "./shared.js"; export function normalizeWhatsAppMessagingTarget(raw: string): string | undefined { - const trimmed = raw.trim(); + const trimmed = trimMessagingTarget(raw); if (!trimmed) { return undefined; } @@ -9,15 +10,8 @@ export function normalizeWhatsAppMessagingTarget(raw: string): string | undefine } export function looksLikeWhatsAppTargetId(raw: string): boolean { - const trimmed = raw.trim(); - if (!trimmed) { - return false; - } - if (/^whatsapp:/i.test(trimmed)) { - return true; - } - if (trimmed.includes("@")) { - return true; - } - return /^\+?\d{3,}$/.test(trimmed); + return looksLikeHandleOrPhoneTarget({ + raw, + prefixPattern: /^whatsapp:/i, + }); } diff --git a/src/channels/plugins/onboarding/channel-access-configure.test.ts b/src/channels/plugins/onboarding/channel-access-configure.test.ts new file mode 100644 index 000000000..aba8f05ea --- /dev/null +++ b/src/channels/plugins/onboarding/channel-access-configure.test.ts @@ -0,0 +1,142 @@ +import { describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../../../config/config.js"; +import { configureChannelAccessWithAllowlist } from "./channel-access-configure.js"; +import type { ChannelAccessPolicy } from "./channel-access.js"; + +function createPrompter(params: { confirm: boolean; policy?: ChannelAccessPolicy; text?: string }) { + return { + confirm: vi.fn(async () => params.confirm), + select: vi.fn(async () => params.policy ?? "allowlist"), + text: vi.fn(async () => params.text ?? ""), + note: vi.fn(), + }; +} + +async function runConfigureChannelAccess(params: { + cfg: OpenClawConfig; + prompter: ReturnType; + label?: string; + placeholder?: string; + setPolicy: (cfg: OpenClawConfig, policy: ChannelAccessPolicy) => OpenClawConfig; + resolveAllowlist: (params: { cfg: OpenClawConfig; entries: string[] }) => Promise; + applyAllowlist: (params: { cfg: OpenClawConfig; resolved: TResolved }) => OpenClawConfig; +}) { + return await configureChannelAccessWithAllowlist({ + cfg: params.cfg, + // oxlint-disable-next-line typescript/no-explicit-any + prompter: params.prompter as any, + label: params.label ?? "Slack channels", + currentPolicy: "allowlist", + currentEntries: [], + placeholder: params.placeholder ?? "#general", + updatePrompt: true, + setPolicy: params.setPolicy, + resolveAllowlist: params.resolveAllowlist, + applyAllowlist: params.applyAllowlist, + }); +} + +describe("configureChannelAccessWithAllowlist", () => { + it("returns input config when user skips access configuration", async () => { + const cfg: OpenClawConfig = {}; + const prompter = createPrompter({ confirm: false }); + const setPolicy = vi.fn((next: OpenClawConfig) => next); + const resolveAllowlist = vi.fn(async () => [] as string[]); + const applyAllowlist = vi.fn((params: { cfg: OpenClawConfig }) => params.cfg); + + const next = await runConfigureChannelAccess({ + cfg, + prompter, + setPolicy, + resolveAllowlist, + applyAllowlist, + }); + + expect(next).toBe(cfg); + expect(setPolicy).not.toHaveBeenCalled(); + expect(resolveAllowlist).not.toHaveBeenCalled(); + expect(applyAllowlist).not.toHaveBeenCalled(); + }); + + it("applies non-allowlist policy directly", async () => { + const cfg: OpenClawConfig = {}; + const prompter = createPrompter({ + confirm: true, + policy: "open", + }); + const setPolicy = vi.fn( + (next: OpenClawConfig, policy: ChannelAccessPolicy): OpenClawConfig => ({ + ...next, + channels: { discord: { groupPolicy: policy } }, + }), + ); + const resolveAllowlist = vi.fn(async () => ["ignored"]); + const applyAllowlist = vi.fn((params: { cfg: OpenClawConfig }) => params.cfg); + + const next = await runConfigureChannelAccess({ + cfg, + prompter, + label: "Discord channels", + placeholder: "guild/channel", + setPolicy, + resolveAllowlist, + applyAllowlist, + }); + + expect(next.channels?.discord?.groupPolicy).toBe("open"); + expect(setPolicy).toHaveBeenCalledWith(cfg, "open"); + expect(resolveAllowlist).not.toHaveBeenCalled(); + expect(applyAllowlist).not.toHaveBeenCalled(); + }); + + it("resolves allowlist entries and applies them after forcing allowlist policy", async () => { + const cfg: OpenClawConfig = {}; + const prompter = createPrompter({ + confirm: true, + policy: "allowlist", + text: "#general, #support", + }); + const calls: string[] = []; + const setPolicy = vi.fn((next: OpenClawConfig, policy: ChannelAccessPolicy): OpenClawConfig => { + calls.push("setPolicy"); + return { + ...next, + channels: { slack: { groupPolicy: policy } }, + }; + }); + const resolveAllowlist = vi.fn(async (params: { cfg: OpenClawConfig; entries: string[] }) => { + calls.push("resolve"); + expect(params.cfg).toBe(cfg); + expect(params.entries).toEqual(["#general", "#support"]); + return ["C1", "C2"]; + }); + const applyAllowlist = vi.fn((params: { cfg: OpenClawConfig; resolved: string[] }) => { + calls.push("apply"); + expect(params.cfg.channels?.slack?.groupPolicy).toBe("allowlist"); + return { + ...params.cfg, + channels: { + ...params.cfg.channels, + slack: { + ...params.cfg.channels?.slack, + channels: Object.fromEntries(params.resolved.map((id) => [id, { allow: true }])), + }, + }, + }; + }); + + const next = await runConfigureChannelAccess({ + cfg, + prompter, + setPolicy, + resolveAllowlist, + applyAllowlist, + }); + + expect(calls).toEqual(["resolve", "setPolicy", "apply"]); + expect(next.channels?.slack?.channels).toEqual({ + C1: { allow: true }, + C2: { allow: true }, + }); + }); +}); diff --git a/src/channels/plugins/onboarding/channel-access-configure.ts b/src/channels/plugins/onboarding/channel-access-configure.ts new file mode 100644 index 000000000..200efce58 --- /dev/null +++ b/src/channels/plugins/onboarding/channel-access-configure.ts @@ -0,0 +1,41 @@ +import type { OpenClawConfig } from "../../../config/config.js"; +import type { WizardPrompter } from "../../../wizard/prompts.js"; +import { promptChannelAccessConfig, type ChannelAccessPolicy } from "./channel-access.js"; + +export async function configureChannelAccessWithAllowlist(params: { + cfg: OpenClawConfig; + prompter: WizardPrompter; + label: string; + currentPolicy: ChannelAccessPolicy; + currentEntries: string[]; + placeholder: string; + updatePrompt: boolean; + setPolicy: (cfg: OpenClawConfig, policy: ChannelAccessPolicy) => OpenClawConfig; + resolveAllowlist: (params: { cfg: OpenClawConfig; entries: string[] }) => Promise; + applyAllowlist: (params: { cfg: OpenClawConfig; resolved: TResolved }) => OpenClawConfig; +}): Promise { + let next = params.cfg; + const accessConfig = await promptChannelAccessConfig({ + prompter: params.prompter, + label: params.label, + currentPolicy: params.currentPolicy, + currentEntries: params.currentEntries, + placeholder: params.placeholder, + updatePrompt: params.updatePrompt, + }); + if (!accessConfig) { + return next; + } + if (accessConfig.policy !== "allowlist") { + return params.setPolicy(next, accessConfig.policy); + } + const resolved = await params.resolveAllowlist({ + cfg: next, + entries: accessConfig.entries, + }); + next = params.setPolicy(next, "allowlist"); + return params.applyAllowlist({ + cfg: next, + resolved, + }); +} diff --git a/src/channels/plugins/onboarding/discord.ts b/src/channels/plugins/onboarding/discord.ts index 9009f528e..2eebe7a76 100644 --- a/src/channels/plugins/onboarding/discord.ts +++ b/src/channels/plugins/onboarding/discord.ts @@ -1,6 +1,5 @@ import type { OpenClawConfig } from "../../../config/config.js"; import type { DiscordGuildEntry } from "../../../config/types.discord.js"; -import type { DmPolicy } from "../../../config/types.js"; import { listDiscordAccountIds, resolveDefaultDiscordAccountId, @@ -16,38 +15,24 @@ import { DEFAULT_ACCOUNT_ID } from "../../../routing/session-key.js"; import { formatDocsLink } from "../../../terminal/links.js"; import type { WizardPrompter } from "../../../wizard/prompts.js"; import type { ChannelOnboardingAdapter, ChannelOnboardingDmPolicy } from "../onboarding-types.js"; -import { promptChannelAccessConfig } from "./channel-access.js"; +import { configureChannelAccessWithAllowlist } from "./channel-access-configure.js"; import { - addWildcardAllowFrom, - promptResolvedAllowFrom, + applySingleTokenPromptResult, + parseMentionOrPrefixedId, + noteChannelLookupFailure, + noteChannelLookupSummary, + patchChannelConfigForAccount, + promptLegacyChannelAllowFrom, + promptSingleChannelToken, resolveAccountIdForConfigure, resolveOnboardingAccountId, - splitOnboardingEntries, + setAccountGroupPolicyForChannel, + setLegacyChannelDmPolicyWithAllowFrom, + setOnboardingChannelEnabled, } from "./helpers.js"; const channel = "discord" as const; -function setDiscordDmPolicy(cfg: OpenClawConfig, dmPolicy: DmPolicy) { - const existingAllowFrom = - cfg.channels?.discord?.allowFrom ?? cfg.channels?.discord?.dm?.allowFrom; - const allowFrom = dmPolicy === "open" ? addWildcardAllowFrom(existingAllowFrom) : undefined; - return { - ...cfg, - channels: { - ...cfg.channels, - discord: { - ...cfg.channels?.discord, - dmPolicy, - ...(allowFrom ? { allowFrom } : {}), - dm: { - ...cfg.channels?.discord?.dm, - enabled: cfg.channels?.discord?.dm?.enabled ?? true, - }, - }, - }, - }; -} - async function noteDiscordTokenHelp(prompter: WizardPrompter): Promise { await prompter.note( [ @@ -61,52 +46,6 @@ async function noteDiscordTokenHelp(prompter: WizardPrompter): Promise { ); } -function patchDiscordConfigForAccount( - cfg: OpenClawConfig, - accountId: string, - patch: Record, -): OpenClawConfig { - if (accountId === DEFAULT_ACCOUNT_ID) { - return { - ...cfg, - channels: { - ...cfg.channels, - discord: { - ...cfg.channels?.discord, - enabled: true, - ...patch, - }, - }, - }; - } - return { - ...cfg, - channels: { - ...cfg.channels, - discord: { - ...cfg.channels?.discord, - enabled: true, - accounts: { - ...cfg.channels?.discord?.accounts, - [accountId]: { - ...cfg.channels?.discord?.accounts?.[accountId], - enabled: cfg.channels?.discord?.accounts?.[accountId]?.enabled ?? true, - ...patch, - }, - }, - }, - }, - }; -} - -function setDiscordGroupPolicy( - cfg: OpenClawConfig, - accountId: string, - groupPolicy: "open" | "allowlist" | "disabled", -): OpenClawConfig { - return patchDiscordConfigForAccount(cfg, accountId, { groupPolicy }); -} - function setDiscordGuildChannelAllowlist( cfg: OpenClawConfig, accountId: string, @@ -131,24 +70,12 @@ function setDiscordGuildChannelAllowlist( guilds[guildKey] = existing; } } - return patchDiscordConfigForAccount(cfg, accountId, { guilds }); -} - -function setDiscordAllowFrom(cfg: OpenClawConfig, allowFrom: string[]): OpenClawConfig { - return { - ...cfg, - channels: { - ...cfg.channels, - discord: { - ...cfg.channels?.discord, - allowFrom, - dm: { - ...cfg.channels?.discord?.dm, - enabled: cfg.channels?.discord?.dm?.enabled ?? true, - }, - }, - }, - }; + return patchChannelConfigForAccount({ + cfg, + channel: "discord", + accountId, + patch: { guilds }, + }); } async function promptDiscordAllowFrom(params: { @@ -164,8 +91,22 @@ async function promptDiscordAllowFrom(params: { const token = resolved.token; const existing = params.cfg.channels?.discord?.allowFrom ?? params.cfg.channels?.discord?.dm?.allowFrom ?? []; - await params.prompter.note( - [ + const parseId = (value: string) => + parseMentionOrPrefixedId({ + value, + mentionPattern: /^<@!?(\d+)>$/, + prefixPattern: /^(user:|discord:)/i, + idPattern: /^\d+$/, + }); + + return promptLegacyChannelAllowFrom({ + cfg: params.cfg, + channel: "discord", + prompter: params.prompter, + existing, + token, + noteTitle: "Discord allowlist", + noteLines: [ "Allowlist Discord DMs by username (we resolve to user ids).", "Examples:", "- 123456789012345678", @@ -173,35 +114,9 @@ async function promptDiscordAllowFrom(params: { "- alice#1234", "Multiple entries: comma-separated.", `Docs: ${formatDocsLink("/discord", "discord")}`, - ].join("\n"), - "Discord allowlist", - ); - - const parseInputs = (value: string) => splitOnboardingEntries(value); - const parseId = (value: string) => { - const trimmed = value.trim(); - if (!trimmed) { - return null; - } - const mention = trimmed.match(/^<@!?(\d+)>$/); - if (mention) { - return mention[1]; - } - const prefixed = trimmed.replace(/^(user:|discord:)/i, ""); - if (/^\d+$/.test(prefixed)) { - return prefixed; - } - return null; - }; - - const unique = await promptResolvedAllowFrom({ - prompter: params.prompter, - existing, - token, + ], message: "Discord allowFrom (usernames or ids)", placeholder: "@alice, 123456789012345678", - label: "Discord allowlist", - parseInputs, parseId, invalidWithoutTokenNote: "Bot token missing; use numeric user ids (or mention form) only.", resolveEntries: ({ token, entries }) => @@ -210,7 +125,6 @@ async function promptDiscordAllowFrom(params: { entries, }), }); - return setDiscordAllowFrom(params.cfg, unique); } const dmPolicy: ChannelOnboardingDmPolicy = { @@ -220,7 +134,12 @@ const dmPolicy: ChannelOnboardingDmPolicy = { allowFromKey: "channels.discord.allowFrom", getCurrent: (cfg) => cfg.channels?.discord?.dmPolicy ?? cfg.channels?.discord?.dm?.policy ?? "pairing", - setPolicy: (cfg, policy) => setDiscordDmPolicy(cfg, policy), + setPolicy: (cfg, policy) => + setLegacyChannelDmPolicyWithAllowFrom({ + cfg, + channel: "discord", + dmPolicy: policy, + }), promptAllowFrom: promptDiscordAllowFrom, }; @@ -257,86 +176,31 @@ export const discordOnboardingAdapter: ChannelOnboardingAdapter = { }); const accountConfigured = Boolean(resolvedAccount.token); const allowEnv = discordAccountId === DEFAULT_ACCOUNT_ID; - const canUseEnv = allowEnv && Boolean(process.env.DISCORD_BOT_TOKEN?.trim()); + const canUseEnv = + allowEnv && !resolvedAccount.config.token && Boolean(process.env.DISCORD_BOT_TOKEN?.trim()); const hasConfigToken = Boolean(resolvedAccount.config.token); - let token: string | null = null; if (!accountConfigured) { await noteDiscordTokenHelp(prompter); } - if (canUseEnv && !resolvedAccount.config.token) { - const keepEnv = await prompter.confirm({ - message: "DISCORD_BOT_TOKEN detected. Use env var?", - initialValue: true, - }); - if (keepEnv) { - next = { - ...next, - channels: { - ...next.channels, - discord: { ...next.channels?.discord, enabled: true }, - }, - }; - } else { - token = String( - await prompter.text({ - message: "Enter Discord bot token", - validate: (value) => (value?.trim() ? undefined : "Required"), - }), - ).trim(); - } - } else if (hasConfigToken) { - const keep = await prompter.confirm({ - message: "Discord token already configured. Keep it?", - initialValue: true, - }); - if (!keep) { - token = String( - await prompter.text({ - message: "Enter Discord bot token", - validate: (value) => (value?.trim() ? undefined : "Required"), - }), - ).trim(); - } - } else { - token = String( - await prompter.text({ - message: "Enter Discord bot token", - validate: (value) => (value?.trim() ? undefined : "Required"), - }), - ).trim(); - } - if (token) { - if (discordAccountId === DEFAULT_ACCOUNT_ID) { - next = { - ...next, - channels: { - ...next.channels, - discord: { ...next.channels?.discord, enabled: true, token }, - }, - }; - } else { - next = { - ...next, - channels: { - ...next.channels, - discord: { - ...next.channels?.discord, - enabled: true, - accounts: { - ...next.channels?.discord?.accounts, - [discordAccountId]: { - ...next.channels?.discord?.accounts?.[discordAccountId], - enabled: next.channels?.discord?.accounts?.[discordAccountId]?.enabled ?? true, - token, - }, - }, - }, - }, - }; - } - } + const tokenResult = await promptSingleChannelToken({ + prompter, + accountConfigured, + canUseEnv, + hasConfigToken, + envPrompt: "DISCORD_BOT_TOKEN detected. Use env var?", + keepPrompt: "Discord token already configured. Keep it?", + inputPrompt: "Enter Discord bot token", + }); + + next = applySingleTokenPromptResult({ + cfg: next, + channel: "discord", + accountId: discordAccountId, + tokenPatchKey: "token", + tokenResult, + }); const currentEntries = Object.entries(resolvedAccount.config.guilds ?? {}).flatMap( ([guildKey, value]) => { @@ -349,31 +213,35 @@ export const discordOnboardingAdapter: ChannelOnboardingAdapter = { return channelKeys.map((channelKey) => `${guildKey}/${channelKey}`); }, ); - const accessConfig = await promptChannelAccessConfig({ + next = await configureChannelAccessWithAllowlist({ + cfg: next, prompter, label: "Discord channels", currentPolicy: resolvedAccount.config.groupPolicy ?? "allowlist", currentEntries, placeholder: "My Server/#general, guildId/channelId, #support", updatePrompt: Boolean(resolvedAccount.config.guilds), - }); - if (accessConfig) { - if (accessConfig.policy !== "allowlist") { - next = setDiscordGroupPolicy(next, discordAccountId, accessConfig.policy); - } else { + setPolicy: (cfg, policy) => + setAccountGroupPolicyForChannel({ + cfg, + channel: "discord", + accountId: discordAccountId, + groupPolicy: policy, + }), + resolveAllowlist: async ({ cfg, entries }) => { const accountWithTokens = resolveDiscordAccount({ - cfg: next, + cfg, accountId: discordAccountId, }); - let resolved: DiscordChannelResolution[] = accessConfig.entries.map((input) => ({ + let resolved: DiscordChannelResolution[] = entries.map((input) => ({ input, resolved: false, })); - if (accountWithTokens.token && accessConfig.entries.length > 0) { + if (accountWithTokens.token && entries.length > 0) { try { resolved = await resolveDiscordChannelAllowlist({ token: accountWithTokens.token, - entries: accessConfig.entries, + entries, }); const resolvedChannels = resolved.filter((entry) => entry.resolved && entry.channelId); const resolvedGuilds = resolved.filter( @@ -382,36 +250,36 @@ export const discordOnboardingAdapter: ChannelOnboardingAdapter = { const unresolved = resolved .filter((entry) => !entry.resolved) .map((entry) => entry.input); - if (resolvedChannels.length > 0 || resolvedGuilds.length > 0 || unresolved.length > 0) { - const summary: string[] = []; - if (resolvedChannels.length > 0) { - summary.push( - `Resolved channels: ${resolvedChannels + await noteChannelLookupSummary({ + prompter, + label: "Discord channels", + resolvedSections: [ + { + title: "Resolved channels", + values: resolvedChannels .map((entry) => entry.channelId) - .filter(Boolean) - .join(", ")}`, - ); - } - if (resolvedGuilds.length > 0) { - summary.push( - `Resolved guilds: ${resolvedGuilds + .filter((value): value is string => Boolean(value)), + }, + { + title: "Resolved guilds", + values: resolvedGuilds .map((entry) => entry.guildId) - .filter(Boolean) - .join(", ")}`, - ); - } - if (unresolved.length > 0) { - summary.push(`Unresolved (kept as typed): ${unresolved.join(", ")}`); - } - await prompter.note(summary.join("\n"), "Discord channels"); - } + .filter((value): value is string => Boolean(value)), + }, + ], + unresolved, + }); } catch (err) { - await prompter.note( - `Channel lookup failed; keeping entries as typed. ${String(err)}`, - "Discord channels", - ); + await noteChannelLookupFailure({ + prompter, + label: "Discord channels", + error: err, + }); } } + return resolved; + }, + applyAllowlist: ({ cfg, resolved }) => { const allowlistEntries: Array<{ guildKey: string; channelKey?: string }> = []; for (const entry of resolved) { const guildKey = @@ -426,19 +294,12 @@ export const discordOnboardingAdapter: ChannelOnboardingAdapter = { } allowlistEntries.push({ guildKey, ...(channelKey ? { channelKey } : {}) }); } - next = setDiscordGroupPolicy(next, discordAccountId, "allowlist"); - next = setDiscordGuildChannelAllowlist(next, discordAccountId, allowlistEntries); - } - } + return setDiscordGuildChannelAllowlist(cfg, discordAccountId, allowlistEntries); + }, + }); return { cfg: next, accountId: discordAccountId }; }, dmPolicy, - disable: (cfg) => ({ - ...cfg, - channels: { - ...cfg.channels, - discord: { ...cfg.channels?.discord, enabled: false }, - }, - }), + disable: (cfg) => setOnboardingChannelEnabled(cfg, channel, false), }; diff --git a/src/channels/plugins/onboarding/helpers.test.ts b/src/channels/plugins/onboarding/helpers.test.ts index 2ff9b2967..cecb55181 100644 --- a/src/channels/plugins/onboarding/helpers.test.ts +++ b/src/channels/plugins/onboarding/helpers.test.ts @@ -8,12 +8,27 @@ vi.mock("../../../plugin-sdk/onboarding.js", () => ({ })); import { + applySingleTokenPromptResult, normalizeAllowFromEntries, + noteChannelLookupFailure, + noteChannelLookupSummary, + parseMentionOrPrefixedId, + parseOnboardingEntriesAllowingWildcard, + patchChannelConfigForAccount, + patchLegacyDmChannelConfig, + promptLegacyChannelAllowFrom, + parseOnboardingEntriesWithParser, + promptParsedAllowFromForScopedChannel, + promptSingleChannelToken, promptResolvedAllowFrom, resolveAccountIdForConfigure, resolveOnboardingAccountId, setAccountAllowFromForChannel, + setAccountGroupPolicyForChannel, setChannelDmPolicyWithAllowFrom, + setLegacyChannelAllowFrom, + setLegacyChannelDmPolicyWithAllowFrom, + setOnboardingChannelEnabled, splitOnboardingEntries, } from "./helpers.js"; @@ -24,6 +39,95 @@ function createPrompter(inputs: string[]) { }; } +function createTokenPrompter(params: { confirms: boolean[]; texts: string[] }) { + const confirms = [...params.confirms]; + const texts = [...params.texts]; + return { + confirm: vi.fn(async () => confirms.shift() ?? true), + text: vi.fn(async () => texts.shift() ?? ""), + }; +} + +function parseCsvInputs(value: string): string[] { + return value + .split(",") + .map((part) => part.trim()) + .filter(Boolean); +} + +type AllowFromResolver = (params: { + token: string; + entries: string[]; +}) => Promise>; + +function asAllowFromResolver(resolveEntries: ReturnType): AllowFromResolver { + return resolveEntries as AllowFromResolver; +} + +async function runPromptResolvedAllowFromWithToken(params: { + prompter: ReturnType; + resolveEntries: AllowFromResolver; +}) { + return await promptResolvedAllowFrom({ + // oxlint-disable-next-line typescript/no-explicit-any + prompter: params.prompter as any, + existing: [], + token: "xoxb-test", + message: "msg", + placeholder: "placeholder", + label: "allowlist", + parseInputs: parseCsvInputs, + parseId: () => null, + invalidWithoutTokenNote: "ids only", + resolveEntries: params.resolveEntries, + }); +} + +async function runPromptSingleToken(params: { + prompter: ReturnType; + accountConfigured: boolean; + canUseEnv: boolean; + hasConfigToken: boolean; +}) { + return await promptSingleChannelToken({ + prompter: params.prompter, + accountConfigured: params.accountConfigured, + canUseEnv: params.canUseEnv, + hasConfigToken: params.hasConfigToken, + envPrompt: "use env", + keepPrompt: "keep", + inputPrompt: "token", + }); +} + +async function runPromptLegacyAllowFrom(params: { + cfg?: OpenClawConfig; + channel: "discord" | "slack"; + prompter: ReturnType; + existing: string[]; + token: string; + noteTitle: string; + noteLines: string[]; + parseId: (value: string) => string | null; + resolveEntries: AllowFromResolver; +}) { + return await promptLegacyChannelAllowFrom({ + cfg: params.cfg ?? {}, + channel: params.channel, + // oxlint-disable-next-line typescript/no-explicit-any + prompter: params.prompter as any, + existing: params.existing, + token: params.token, + noteTitle: params.noteTitle, + noteLines: params.noteLines, + message: "msg", + placeholder: "placeholder", + parseId: params.parseId, + invalidWithoutTokenNote: "ids only", + resolveEntries: params.resolveEntries, + }); +} + describe("promptResolvedAllowFrom", () => { beforeEach(() => { promptAccountIdSdkMock.mockReset(); @@ -42,11 +146,7 @@ describe("promptResolvedAllowFrom", () => { message: "msg", placeholder: "placeholder", label: "allowlist", - parseInputs: (value) => - value - .split(",") - .map((part) => part.trim()) - .filter(Boolean), + parseInputs: parseCsvInputs, parseId: (value) => (/^\d+$/.test(value.trim()) ? value.trim() : null), invalidWithoutTokenNote: "ids only", // oxlint-disable-next-line typescript/no-explicit-any @@ -65,22 +165,9 @@ describe("promptResolvedAllowFrom", () => { .mockResolvedValueOnce([{ input: "alice", resolved: false }]) .mockResolvedValueOnce([{ input: "bob", resolved: true, id: "U123" }]); - const result = await promptResolvedAllowFrom({ - // oxlint-disable-next-line typescript/no-explicit-any - prompter: prompter as any, - existing: [], - token: "xoxb-test", - message: "msg", - placeholder: "placeholder", - label: "allowlist", - parseInputs: (value) => - value - .split(",") - .map((part) => part.trim()) - .filter(Boolean), - parseId: () => null, - invalidWithoutTokenNote: "ids only", - resolveEntries, + const result = await runPromptResolvedAllowFromWithToken({ + prompter, + resolveEntries: asAllowFromResolver(resolveEntries), }); expect(result).toEqual(["U123"]); @@ -95,22 +182,9 @@ describe("promptResolvedAllowFrom", () => { .mockRejectedValueOnce(new Error("network")) .mockResolvedValueOnce([{ input: "bob", resolved: true, id: "U234" }]); - const result = await promptResolvedAllowFrom({ - // oxlint-disable-next-line typescript/no-explicit-any - prompter: prompter as any, - existing: [], - token: "xoxb-test", - message: "msg", - placeholder: "placeholder", - label: "allowlist", - parseInputs: (value) => - value - .split(",") - .map((part) => part.trim()) - .filter(Boolean), - parseId: () => null, - invalidWithoutTokenNote: "ids only", - resolveEntries, + const result = await runPromptResolvedAllowFromWithToken({ + prompter, + resolveEntries: asAllowFromResolver(resolveEntries), }); expect(result).toEqual(["U234"]); @@ -122,6 +196,262 @@ describe("promptResolvedAllowFrom", () => { }); }); +describe("promptLegacyChannelAllowFrom", () => { + it("applies parsed ids without token resolution", async () => { + const prompter = createPrompter([" 123 "]); + const resolveEntries = vi.fn(); + + const next = await runPromptLegacyAllowFrom({ + cfg: {} as OpenClawConfig, + channel: "discord", + existing: ["999"], + prompter, + token: "", + noteTitle: "Discord allowlist", + noteLines: ["line1", "line2"], + parseId: (value) => (/^\d+$/.test(value.trim()) ? value.trim() : null), + resolveEntries: asAllowFromResolver(resolveEntries), + }); + + expect(next.channels?.discord?.allowFrom).toEqual(["999", "123"]); + expect(prompter.note).toHaveBeenCalledWith("line1\nline2", "Discord allowlist"); + expect(resolveEntries).not.toHaveBeenCalled(); + }); + + it("uses resolver when token is present", async () => { + const prompter = createPrompter(["alice"]); + const resolveEntries = vi.fn(async () => [{ input: "alice", resolved: true, id: "U1" }]); + + const next = await runPromptLegacyAllowFrom({ + cfg: {} as OpenClawConfig, + channel: "slack", + prompter, + existing: [], + token: "xoxb-token", + noteTitle: "Slack allowlist", + noteLines: ["line"], + parseId: () => null, + resolveEntries: asAllowFromResolver(resolveEntries), + }); + + expect(next.channels?.slack?.allowFrom).toEqual(["U1"]); + expect(resolveEntries).toHaveBeenCalledWith({ token: "xoxb-token", entries: ["alice"] }); + }); +}); + +describe("promptSingleChannelToken", () => { + it("uses env tokens when confirmed", async () => { + const prompter = createTokenPrompter({ confirms: [true], texts: [] }); + const result = await runPromptSingleToken({ + prompter, + accountConfigured: false, + canUseEnv: true, + hasConfigToken: false, + }); + expect(result).toEqual({ useEnv: true, token: null }); + expect(prompter.text).not.toHaveBeenCalled(); + }); + + it("prompts for token when env exists but user declines env", async () => { + const prompter = createTokenPrompter({ confirms: [false], texts: ["abc"] }); + const result = await runPromptSingleToken({ + prompter, + accountConfigured: false, + canUseEnv: true, + hasConfigToken: false, + }); + expect(result).toEqual({ useEnv: false, token: "abc" }); + }); + + it("keeps existing configured token when confirmed", async () => { + const prompter = createTokenPrompter({ confirms: [true], texts: [] }); + const result = await runPromptSingleToken({ + prompter, + accountConfigured: true, + canUseEnv: false, + hasConfigToken: true, + }); + expect(result).toEqual({ useEnv: false, token: null }); + expect(prompter.text).not.toHaveBeenCalled(); + }); + + it("prompts for token when no env/config token is used", async () => { + const prompter = createTokenPrompter({ confirms: [false], texts: ["xyz"] }); + const result = await runPromptSingleToken({ + prompter, + accountConfigured: true, + canUseEnv: false, + hasConfigToken: false, + }); + expect(result).toEqual({ useEnv: false, token: "xyz" }); + }); +}); + +describe("applySingleTokenPromptResult", () => { + it("writes env selection as an empty patch on target account", () => { + const next = applySingleTokenPromptResult({ + cfg: {}, + channel: "discord", + accountId: "work", + tokenPatchKey: "token", + tokenResult: { useEnv: true, token: null }, + }); + + expect(next.channels?.discord?.enabled).toBe(true); + expect(next.channels?.discord?.accounts?.work?.enabled).toBe(true); + expect(next.channels?.discord?.accounts?.work?.token).toBeUndefined(); + }); + + it("writes provided token under requested key", () => { + const next = applySingleTokenPromptResult({ + cfg: {}, + channel: "telegram", + accountId: DEFAULT_ACCOUNT_ID, + tokenPatchKey: "botToken", + tokenResult: { useEnv: false, token: "abc" }, + }); + + expect(next.channels?.telegram?.enabled).toBe(true); + expect(next.channels?.telegram?.botToken).toBe("abc"); + }); +}); + +describe("promptParsedAllowFromForScopedChannel", () => { + it("writes parsed allowFrom values to default account channel config", async () => { + const cfg: OpenClawConfig = { + channels: { + imessage: { + allowFrom: ["old"], + }, + }, + }; + const prompter = createPrompter([" Alice, ALICE "]); + + const next = await promptParsedAllowFromForScopedChannel({ + cfg, + channel: "imessage", + defaultAccountId: DEFAULT_ACCOUNT_ID, + prompter, + noteTitle: "iMessage allowlist", + noteLines: ["line1", "line2"], + message: "msg", + placeholder: "placeholder", + parseEntries: (raw) => + parseOnboardingEntriesWithParser(raw, (entry) => ({ value: entry.toLowerCase() })), + getExistingAllowFrom: ({ cfg }) => cfg.channels?.imessage?.allowFrom ?? [], + }); + + expect(next.channels?.imessage?.allowFrom).toEqual(["alice"]); + expect(prompter.note).toHaveBeenCalledWith("line1\nline2", "iMessage allowlist"); + }); + + it("writes parsed values to non-default account allowFrom", async () => { + const cfg: OpenClawConfig = { + channels: { + signal: { + accounts: { + alt: { + allowFrom: ["+15555550123"], + }, + }, + }, + }, + }; + const prompter = createPrompter(["+15555550124"]); + + const next = await promptParsedAllowFromForScopedChannel({ + cfg, + channel: "signal", + accountId: "alt", + defaultAccountId: DEFAULT_ACCOUNT_ID, + prompter, + noteTitle: "Signal allowlist", + noteLines: ["line"], + message: "msg", + placeholder: "placeholder", + parseEntries: (raw) => ({ entries: [raw.trim()] }), + getExistingAllowFrom: ({ cfg, accountId }) => + cfg.channels?.signal?.accounts?.[accountId]?.allowFrom ?? [], + }); + + expect(next.channels?.signal?.accounts?.alt?.allowFrom).toEqual(["+15555550124"]); + expect(next.channels?.signal?.allowFrom).toBeUndefined(); + }); + + it("uses parser validation from the prompt validate callback", async () => { + const prompter = { + note: vi.fn(async () => undefined), + text: vi.fn(async (params: { validate?: (value: string) => string | undefined }) => { + expect(params.validate?.("")).toBe("Required"); + expect(params.validate?.("bad")).toBe("bad entry"); + expect(params.validate?.("ok")).toBeUndefined(); + return "ok"; + }), + }; + + const next = await promptParsedAllowFromForScopedChannel({ + cfg: {}, + channel: "imessage", + defaultAccountId: DEFAULT_ACCOUNT_ID, + prompter, + noteTitle: "title", + noteLines: ["line"], + message: "msg", + placeholder: "placeholder", + parseEntries: (raw) => + raw.trim() === "bad" + ? { entries: [], error: "bad entry" } + : { entries: [raw.trim().toLowerCase()] }, + getExistingAllowFrom: () => [], + }); + + expect(next.channels?.imessage?.allowFrom).toEqual(["ok"]); + }); +}); + +describe("channel lookup note helpers", () => { + it("emits summary lines for resolved and unresolved entries", async () => { + const prompter = { note: vi.fn(async () => undefined) }; + await noteChannelLookupSummary({ + prompter, + label: "Slack channels", + resolvedSections: [ + { title: "Resolved", values: ["C1", "C2"] }, + { title: "Resolved guilds", values: [] }, + ], + unresolved: ["#typed-name"], + }); + expect(prompter.note).toHaveBeenCalledWith( + "Resolved: C1, C2\nUnresolved (kept as typed): #typed-name", + "Slack channels", + ); + }); + + it("skips note output when there is nothing to report", async () => { + const prompter = { note: vi.fn(async () => undefined) }; + await noteChannelLookupSummary({ + prompter, + label: "Discord channels", + resolvedSections: [{ title: "Resolved", values: [] }], + unresolved: [], + }); + expect(prompter.note).not.toHaveBeenCalled(); + }); + + it("formats lookup failures consistently", async () => { + const prompter = { note: vi.fn(async () => undefined) }; + await noteChannelLookupFailure({ + prompter, + label: "Discord channels", + error: new Error("boom"), + }); + expect(prompter.note).toHaveBeenCalledWith( + "Channel lookup failed; keeping entries as typed. Error: boom", + "Discord channels", + ); + }); +}); + describe("setAccountAllowFromForChannel", () => { it("writes allowFrom on default account channel config", () => { const cfg: OpenClawConfig = { @@ -173,6 +503,231 @@ describe("setAccountAllowFromForChannel", () => { }); }); +describe("patchChannelConfigForAccount", () => { + it("patches root channel config for default account", () => { + const cfg: OpenClawConfig = { + channels: { + telegram: { + enabled: false, + botToken: "old", + }, + }, + }; + + const next = patchChannelConfigForAccount({ + cfg, + channel: "telegram", + accountId: DEFAULT_ACCOUNT_ID, + patch: { botToken: "new", dmPolicy: "allowlist" }, + }); + + expect(next.channels?.telegram?.enabled).toBe(true); + expect(next.channels?.telegram?.botToken).toBe("new"); + expect(next.channels?.telegram?.dmPolicy).toBe("allowlist"); + }); + + it("patches nested account config and preserves existing enabled flag", () => { + const cfg: OpenClawConfig = { + channels: { + slack: { + enabled: true, + accounts: { + work: { + enabled: false, + botToken: "old-bot", + }, + }, + }, + }, + }; + + const next = patchChannelConfigForAccount({ + cfg, + channel: "slack", + accountId: "work", + patch: { botToken: "new-bot", appToken: "new-app" }, + }); + + expect(next.channels?.slack?.enabled).toBe(true); + expect(next.channels?.slack?.accounts?.work?.enabled).toBe(false); + expect(next.channels?.slack?.accounts?.work?.botToken).toBe("new-bot"); + expect(next.channels?.slack?.accounts?.work?.appToken).toBe("new-app"); + }); + + it("supports imessage/signal account-scoped channel patches", () => { + const cfg: OpenClawConfig = { + channels: { + signal: { + enabled: false, + accounts: {}, + }, + imessage: { + enabled: false, + }, + }, + }; + + const signalNext = patchChannelConfigForAccount({ + cfg, + channel: "signal", + accountId: "work", + patch: { account: "+15555550123", cliPath: "signal-cli" }, + }); + expect(signalNext.channels?.signal?.enabled).toBe(true); + expect(signalNext.channels?.signal?.accounts?.work?.enabled).toBe(true); + expect(signalNext.channels?.signal?.accounts?.work?.account).toBe("+15555550123"); + + const imessageNext = patchChannelConfigForAccount({ + cfg: signalNext, + channel: "imessage", + accountId: DEFAULT_ACCOUNT_ID, + patch: { cliPath: "imsg" }, + }); + expect(imessageNext.channels?.imessage?.enabled).toBe(true); + expect(imessageNext.channels?.imessage?.cliPath).toBe("imsg"); + }); +}); + +describe("setOnboardingChannelEnabled", () => { + it("updates enabled and keeps existing channel fields", () => { + const cfg: OpenClawConfig = { + channels: { + discord: { + enabled: true, + token: "abc", + }, + }, + }; + + const next = setOnboardingChannelEnabled(cfg, "discord", false); + expect(next.channels?.discord?.enabled).toBe(false); + expect(next.channels?.discord?.token).toBe("abc"); + }); + + it("creates missing channel config with enabled state", () => { + const next = setOnboardingChannelEnabled({}, "signal", true); + expect(next.channels?.signal?.enabled).toBe(true); + }); +}); + +describe("patchLegacyDmChannelConfig", () => { + it("patches discord root config and defaults dm.enabled to true", () => { + const cfg: OpenClawConfig = { + channels: { + discord: { + dmPolicy: "pairing", + }, + }, + }; + + const next = patchLegacyDmChannelConfig({ + cfg, + channel: "discord", + patch: { allowFrom: ["123"] }, + }); + expect(next.channels?.discord?.allowFrom).toEqual(["123"]); + expect(next.channels?.discord?.dm?.enabled).toBe(true); + }); + + it("preserves explicit dm.enabled=false for slack", () => { + const cfg: OpenClawConfig = { + channels: { + slack: { + dm: { + enabled: false, + }, + }, + }, + }; + + const next = patchLegacyDmChannelConfig({ + cfg, + channel: "slack", + patch: { dmPolicy: "open" }, + }); + expect(next.channels?.slack?.dmPolicy).toBe("open"); + expect(next.channels?.slack?.dm?.enabled).toBe(false); + }); +}); + +describe("setLegacyChannelDmPolicyWithAllowFrom", () => { + it("adds wildcard allowFrom for open policy using legacy dm allowFrom fallback", () => { + const cfg: OpenClawConfig = { + channels: { + discord: { + dm: { + enabled: false, + allowFrom: ["123"], + }, + }, + }, + }; + + const next = setLegacyChannelDmPolicyWithAllowFrom({ + cfg, + channel: "discord", + dmPolicy: "open", + }); + expect(next.channels?.discord?.dmPolicy).toBe("open"); + expect(next.channels?.discord?.allowFrom).toEqual(["123", "*"]); + expect(next.channels?.discord?.dm?.enabled).toBe(false); + }); + + it("sets policy without changing allowFrom when not open", () => { + const cfg: OpenClawConfig = { + channels: { + slack: { + allowFrom: ["U1"], + }, + }, + }; + + const next = setLegacyChannelDmPolicyWithAllowFrom({ + cfg, + channel: "slack", + dmPolicy: "pairing", + }); + expect(next.channels?.slack?.dmPolicy).toBe("pairing"); + expect(next.channels?.slack?.allowFrom).toEqual(["U1"]); + }); +}); + +describe("setLegacyChannelAllowFrom", () => { + it("writes allowFrom through legacy dm patching", () => { + const next = setLegacyChannelAllowFrom({ + cfg: {}, + channel: "slack", + allowFrom: ["U123"], + }); + expect(next.channels?.slack?.allowFrom).toEqual(["U123"]); + expect(next.channels?.slack?.dm?.enabled).toBe(true); + }); +}); + +describe("setAccountGroupPolicyForChannel", () => { + it("writes group policy on default account config", () => { + const next = setAccountGroupPolicyForChannel({ + cfg: {}, + channel: "discord", + accountId: DEFAULT_ACCOUNT_ID, + groupPolicy: "open", + }); + expect(next.channels?.discord?.groupPolicy).toBe("open"); + expect(next.channels?.discord?.enabled).toBe(true); + }); + + it("writes group policy on nested non-default account", () => { + const next = setAccountGroupPolicyForChannel({ + cfg: {}, + channel: "slack", + accountId: "work", + groupPolicy: "disabled", + }); + expect(next.channels?.slack?.accounts?.work?.groupPolicy).toBe("disabled"); + expect(next.channels?.slack?.accounts?.work?.enabled).toBe(true); + }); +}); + describe("setChannelDmPolicyWithAllowFrom", () => { it("adds wildcard allowFrom when setting dmPolicy=open", () => { const cfg: OpenClawConfig = { @@ -213,6 +768,25 @@ describe("setChannelDmPolicyWithAllowFrom", () => { expect(next.channels?.imessage?.dmPolicy).toBe("pairing"); expect(next.channels?.imessage?.allowFrom).toEqual(["*"]); }); + + it("supports telegram channel dmPolicy updates", () => { + const cfg: OpenClawConfig = { + channels: { + telegram: { + dmPolicy: "pairing", + allowFrom: ["123"], + }, + }, + }; + + const next = setChannelDmPolicyWithAllowFrom({ + cfg, + channel: "telegram", + dmPolicy: "open", + }); + expect(next.channels?.telegram?.dmPolicy).toBe("open"); + expect(next.channels?.telegram?.allowFrom).toEqual(["123", "*"]); + }); }); describe("splitOnboardingEntries", () => { @@ -221,6 +795,99 @@ describe("splitOnboardingEntries", () => { }); }); +describe("parseOnboardingEntriesWithParser", () => { + it("maps entries and de-duplicates parsed values", () => { + expect( + parseOnboardingEntriesWithParser(" alice, ALICE ; * ", (entry) => { + if (entry === "*") { + return { value: "*" }; + } + return { value: entry.toLowerCase() }; + }), + ).toEqual({ + entries: ["alice", "*"], + }); + }); + + it("returns parser errors and clears parsed entries", () => { + expect( + parseOnboardingEntriesWithParser("ok, bad", (entry) => + entry === "bad" ? { error: "invalid entry: bad" } : { value: entry }, + ), + ).toEqual({ + entries: [], + error: "invalid entry: bad", + }); + }); +}); + +describe("parseOnboardingEntriesAllowingWildcard", () => { + it("preserves wildcard and delegates non-wildcard entries", () => { + expect( + parseOnboardingEntriesAllowingWildcard(" *, Foo ", (entry) => ({ + value: entry.toLowerCase(), + })), + ).toEqual({ + entries: ["*", "foo"], + }); + }); + + it("returns parser errors for non-wildcard entries", () => { + expect( + parseOnboardingEntriesAllowingWildcard("ok,bad", (entry) => + entry === "bad" ? { error: "bad entry" } : { value: entry }, + ), + ).toEqual({ + entries: [], + error: "bad entry", + }); + }); +}); + +describe("parseMentionOrPrefixedId", () => { + it("parses mention ids", () => { + expect( + parseMentionOrPrefixedId({ + value: "<@!123>", + mentionPattern: /^<@!?(\d+)>$/, + prefixPattern: /^(user:|discord:)/i, + idPattern: /^\d+$/, + }), + ).toBe("123"); + }); + + it("parses prefixed ids and normalizes result", () => { + expect( + parseMentionOrPrefixedId({ + value: "slack:u123abc", + mentionPattern: /^<@([A-Z0-9]+)>$/i, + prefixPattern: /^(slack:|user:)/i, + idPattern: /^[A-Z][A-Z0-9]+$/i, + normalizeId: (id) => id.toUpperCase(), + }), + ).toBe("U123ABC"); + }); + + it("returns null for blank or invalid input", () => { + expect( + parseMentionOrPrefixedId({ + value: " ", + mentionPattern: /^<@!?(\d+)>$/, + prefixPattern: /^(user:|discord:)/i, + idPattern: /^\d+$/, + }), + ).toBeNull(); + expect( + parseMentionOrPrefixedId({ + value: "@alice", + mentionPattern: /^<@!?(\d+)>$/, + prefixPattern: /^(user:|discord:)/i, + idPattern: /^\d+$/, + }), + ).toBeNull(); + }); +}); + describe("normalizeAllowFromEntries", () => { it("normalizes values, preserves wildcard, and removes duplicates", () => { expect( diff --git a/src/channels/plugins/onboarding/helpers.ts b/src/channels/plugins/onboarding/helpers.ts index 7b40c49c0..258aa7b67 100644 --- a/src/channels/plugins/onboarding/helpers.ts +++ b/src/channels/plugins/onboarding/helpers.ts @@ -1,5 +1,5 @@ import type { OpenClawConfig } from "../../../config/config.js"; -import type { DmPolicy } from "../../../config/types.js"; +import type { DmPolicy, GroupPolicy } from "../../../config/types.js"; import { promptAccountId as promptAccountIdSdk } from "../../../plugin-sdk/onboarding.js"; import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "../../../routing/session-key.js"; import type { WizardPrompter } from "../../../wizard/prompts.js"; @@ -32,6 +32,61 @@ export function splitOnboardingEntries(raw: string): string[] { .filter(Boolean); } +type ParsedOnboardingEntry = { value: string } | { error: string }; + +export function parseOnboardingEntriesWithParser( + raw: string, + parseEntry: (entry: string) => ParsedOnboardingEntry, +): { entries: string[]; error?: string } { + const parts = splitOnboardingEntries(String(raw ?? "")); + const entries: string[] = []; + for (const part of parts) { + const parsed = parseEntry(part); + if ("error" in parsed) { + return { entries: [], error: parsed.error }; + } + entries.push(parsed.value); + } + return { entries: normalizeAllowFromEntries(entries) }; +} + +export function parseOnboardingEntriesAllowingWildcard( + raw: string, + parseEntry: (entry: string) => ParsedOnboardingEntry, +): { entries: string[]; error?: string } { + return parseOnboardingEntriesWithParser(raw, (entry) => { + if (entry === "*") { + return { value: "*" }; + } + return parseEntry(entry); + }); +} + +export function parseMentionOrPrefixedId(params: { + value: string; + mentionPattern: RegExp; + prefixPattern?: RegExp; + idPattern: RegExp; + normalizeId?: (id: string) => string; +}): string | null { + const trimmed = params.value.trim(); + if (!trimmed) { + return null; + } + + const mentionMatch = trimmed.match(params.mentionPattern); + if (mentionMatch?.[1]) { + return params.normalizeId ? params.normalizeId(mentionMatch[1]) : mentionMatch[1]; + } + + const stripped = params.prefixPattern ? trimmed.replace(params.prefixPattern, "") : trimmed; + if (!params.idPattern.test(stripped)) { + return null; + } + + return params.normalizeId ? params.normalizeId(stripped) : stripped; +} + export function normalizeAllowFromEntries( entries: Array, normalizeEntry?: (value: string) => string | null | undefined, @@ -91,39 +146,18 @@ export function setAccountAllowFromForChannel(params: { allowFrom: string[]; }): OpenClawConfig { const { cfg, channel, accountId, allowFrom } = params; - if (accountId === DEFAULT_ACCOUNT_ID) { - return { - ...cfg, - channels: { - ...cfg.channels, - [channel]: { - ...cfg.channels?.[channel], - allowFrom, - }, - }, - }; - } - return { - ...cfg, - channels: { - ...cfg.channels, - [channel]: { - ...cfg.channels?.[channel], - accounts: { - ...cfg.channels?.[channel]?.accounts, - [accountId]: { - ...cfg.channels?.[channel]?.accounts?.[accountId], - allowFrom, - }, - }, - }, - }, - }; + return patchConfigForScopedAccount({ + cfg, + channel, + accountId, + patch: { allowFrom }, + ensureEnabled: false, + }); } export function setChannelDmPolicyWithAllowFrom(params: { cfg: OpenClawConfig; - channel: "imessage" | "signal"; + channel: "imessage" | "signal" | "telegram"; dmPolicy: DmPolicy; }): OpenClawConfig { const { cfg, channel, dmPolicy } = params; @@ -142,6 +176,321 @@ export function setChannelDmPolicyWithAllowFrom(params: { }; } +export function setLegacyChannelDmPolicyWithAllowFrom(params: { + cfg: OpenClawConfig; + channel: LegacyDmChannel; + dmPolicy: DmPolicy; +}): OpenClawConfig { + const channelConfig = (params.cfg.channels?.[params.channel] as + | { + allowFrom?: Array; + dm?: { allowFrom?: Array }; + } + | undefined) ?? { + allowFrom: undefined, + dm: undefined, + }; + const existingAllowFrom = channelConfig.allowFrom ?? channelConfig.dm?.allowFrom; + const allowFrom = + params.dmPolicy === "open" ? addWildcardAllowFrom(existingAllowFrom) : undefined; + return patchLegacyDmChannelConfig({ + cfg: params.cfg, + channel: params.channel, + patch: { + dmPolicy: params.dmPolicy, + ...(allowFrom ? { allowFrom } : {}), + }, + }); +} + +export function setLegacyChannelAllowFrom(params: { + cfg: OpenClawConfig; + channel: LegacyDmChannel; + allowFrom: string[]; +}): OpenClawConfig { + return patchLegacyDmChannelConfig({ + cfg: params.cfg, + channel: params.channel, + patch: { allowFrom: params.allowFrom }, + }); +} + +export function setAccountGroupPolicyForChannel(params: { + cfg: OpenClawConfig; + channel: "discord" | "slack"; + accountId: string; + groupPolicy: GroupPolicy; +}): OpenClawConfig { + return patchChannelConfigForAccount({ + cfg: params.cfg, + channel: params.channel, + accountId: params.accountId, + patch: { groupPolicy: params.groupPolicy }, + }); +} + +type AccountScopedChannel = "discord" | "slack" | "telegram" | "imessage" | "signal"; +type LegacyDmChannel = "discord" | "slack"; + +export function patchLegacyDmChannelConfig(params: { + cfg: OpenClawConfig; + channel: LegacyDmChannel; + patch: Record; +}): OpenClawConfig { + const { cfg, channel, patch } = params; + const channelConfig = (cfg.channels?.[channel] as Record | undefined) ?? {}; + const dmConfig = (channelConfig.dm as Record | undefined) ?? {}; + return { + ...cfg, + channels: { + ...cfg.channels, + [channel]: { + ...channelConfig, + ...patch, + dm: { + ...dmConfig, + enabled: typeof dmConfig.enabled === "boolean" ? dmConfig.enabled : true, + }, + }, + }, + }; +} + +export function setOnboardingChannelEnabled( + cfg: OpenClawConfig, + channel: AccountScopedChannel, + enabled: boolean, +): OpenClawConfig { + const channelConfig = (cfg.channels?.[channel] as Record | undefined) ?? {}; + return { + ...cfg, + channels: { + ...cfg.channels, + [channel]: { + ...channelConfig, + enabled, + }, + }, + }; +} + +function patchConfigForScopedAccount(params: { + cfg: OpenClawConfig; + channel: AccountScopedChannel; + accountId: string; + patch: Record; + ensureEnabled: boolean; +}): OpenClawConfig { + const { cfg, channel, accountId, patch, ensureEnabled } = params; + const channelConfig = (cfg.channels?.[channel] as Record | undefined) ?? {}; + + if (accountId === DEFAULT_ACCOUNT_ID) { + return { + ...cfg, + channels: { + ...cfg.channels, + [channel]: { + ...channelConfig, + ...(ensureEnabled ? { enabled: true } : {}), + ...patch, + }, + }, + }; + } + + const accounts = + (channelConfig.accounts as Record> | undefined) ?? {}; + const existingAccount = accounts[accountId] ?? {}; + + return { + ...cfg, + channels: { + ...cfg.channels, + [channel]: { + ...channelConfig, + ...(ensureEnabled ? { enabled: true } : {}), + accounts: { + ...accounts, + [accountId]: { + ...existingAccount, + ...(ensureEnabled + ? { + enabled: + typeof existingAccount.enabled === "boolean" ? existingAccount.enabled : true, + } + : {}), + ...patch, + }, + }, + }, + }, + }; +} + +export function patchChannelConfigForAccount(params: { + cfg: OpenClawConfig; + channel: AccountScopedChannel; + accountId: string; + patch: Record; +}): OpenClawConfig { + return patchConfigForScopedAccount({ + ...params, + ensureEnabled: true, + }); +} + +export function applySingleTokenPromptResult(params: { + cfg: OpenClawConfig; + channel: "discord" | "telegram"; + accountId: string; + tokenPatchKey: "token" | "botToken"; + tokenResult: { + useEnv: boolean; + token: string | null; + }; +}): OpenClawConfig { + let next = params.cfg; + if (params.tokenResult.useEnv) { + next = patchChannelConfigForAccount({ + cfg: next, + channel: params.channel, + accountId: params.accountId, + patch: {}, + }); + } + if (params.tokenResult.token) { + next = patchChannelConfigForAccount({ + cfg: next, + channel: params.channel, + accountId: params.accountId, + patch: { [params.tokenPatchKey]: params.tokenResult.token }, + }); + } + return next; +} + +export async function promptSingleChannelToken(params: { + prompter: Pick; + accountConfigured: boolean; + canUseEnv: boolean; + hasConfigToken: boolean; + envPrompt: string; + keepPrompt: string; + inputPrompt: string; +}): Promise<{ useEnv: boolean; token: string | null }> { + const promptToken = async (): Promise => + String( + await params.prompter.text({ + message: params.inputPrompt, + validate: (value) => (value?.trim() ? undefined : "Required"), + }), + ).trim(); + + if (params.canUseEnv) { + const keepEnv = await params.prompter.confirm({ + message: params.envPrompt, + initialValue: true, + }); + if (keepEnv) { + return { useEnv: true, token: null }; + } + return { useEnv: false, token: await promptToken() }; + } + + if (params.hasConfigToken && params.accountConfigured) { + const keep = await params.prompter.confirm({ + message: params.keepPrompt, + initialValue: true, + }); + if (keep) { + return { useEnv: false, token: null }; + } + } + + return { useEnv: false, token: await promptToken() }; +} + +type ParsedAllowFromResult = { entries: string[]; error?: string }; + +export async function promptParsedAllowFromForScopedChannel(params: { + cfg: OpenClawConfig; + channel: "imessage" | "signal"; + accountId?: string; + defaultAccountId: string; + prompter: Pick; + noteTitle: string; + noteLines: string[]; + message: string; + placeholder: string; + parseEntries: (raw: string) => ParsedAllowFromResult; + getExistingAllowFrom: (params: { + cfg: OpenClawConfig; + accountId: string; + }) => Array; +}): Promise { + const accountId = resolveOnboardingAccountId({ + accountId: params.accountId, + defaultAccountId: params.defaultAccountId, + }); + const existing = params.getExistingAllowFrom({ + cfg: params.cfg, + accountId, + }); + await params.prompter.note(params.noteLines.join("\n"), params.noteTitle); + const entry = await params.prompter.text({ + message: params.message, + placeholder: params.placeholder, + initialValue: existing[0] ? String(existing[0]) : undefined, + validate: (value) => { + const raw = String(value ?? "").trim(); + if (!raw) { + return "Required"; + } + return params.parseEntries(raw).error; + }, + }); + const parsed = params.parseEntries(String(entry)); + const unique = mergeAllowFromEntries(undefined, parsed.entries); + return setAccountAllowFromForChannel({ + cfg: params.cfg, + channel: params.channel, + accountId, + allowFrom: unique, + }); +} + +export async function noteChannelLookupSummary(params: { + prompter: Pick; + label: string; + resolvedSections: Array<{ title: string; values: string[] }>; + unresolved?: string[]; +}): Promise { + const lines: string[] = []; + for (const section of params.resolvedSections) { + if (section.values.length === 0) { + continue; + } + lines.push(`${section.title}: ${section.values.join(", ")}`); + } + if (params.unresolved && params.unresolved.length > 0) { + lines.push(`Unresolved (kept as typed): ${params.unresolved.join(", ")}`); + } + if (lines.length > 0) { + await params.prompter.note(lines.join("\n"), params.label); + } +} + +export async function noteChannelLookupFailure(params: { + prompter: Pick; + label: string; + error: unknown; +}): Promise { + await params.prompter.note( + `Channel lookup failed; keeping entries as typed. ${String(params.error)}`, + params.label, + ); +} + type AllowFromResolution = { input: string; resolved: boolean; @@ -199,3 +548,37 @@ export async function promptResolvedAllowFrom(params: { return mergeAllowFromEntries(params.existing, ids); } } + +export async function promptLegacyChannelAllowFrom(params: { + cfg: OpenClawConfig; + channel: LegacyDmChannel; + prompter: WizardPrompter; + existing: Array; + token?: string | null; + noteTitle: string; + noteLines: string[]; + message: string; + placeholder: string; + parseId: (value: string) => string | null; + invalidWithoutTokenNote: string; + resolveEntries: (params: { token: string; entries: string[] }) => Promise; +}): Promise { + await params.prompter.note(params.noteLines.join("\n"), params.noteTitle); + const unique = await promptResolvedAllowFrom({ + prompter: params.prompter, + existing: params.existing, + token: params.token, + message: params.message, + placeholder: params.placeholder, + label: params.noteTitle, + parseInputs: splitOnboardingEntries, + parseId: params.parseId, + invalidWithoutTokenNote: params.invalidWithoutTokenNote, + resolveEntries: params.resolveEntries, + }); + return setLegacyChannelAllowFrom({ + cfg: params.cfg, + channel: params.channel, + allowFrom: unique, + }); +} diff --git a/src/channels/plugins/onboarding/imessage.test.ts b/src/channels/plugins/onboarding/imessage.test.ts new file mode 100644 index 000000000..266408a61 --- /dev/null +++ b/src/channels/plugins/onboarding/imessage.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from "vitest"; +import { parseIMessageAllowFromEntries } from "./imessage.js"; + +describe("parseIMessageAllowFromEntries", () => { + it("parses handles and chat targets", () => { + expect(parseIMessageAllowFromEntries("+15555550123, chat_id:123, chat_guid:abc")).toEqual({ + entries: ["+15555550123", "chat_id:123", "chat_guid:abc"], + }); + }); + + it("returns validation errors for invalid chat_id", () => { + expect(parseIMessageAllowFromEntries("chat_id:abc")).toEqual({ + entries: [], + error: "Invalid chat_id: chat_id:abc", + }); + }); + + it("returns validation errors for invalid chat_identifier entries", () => { + expect(parseIMessageAllowFromEntries("chat_identifier:")).toEqual({ + entries: [], + error: "Invalid chat_identifier entry", + }); + }); +}); diff --git a/src/channels/plugins/onboarding/imessage.ts b/src/channels/plugins/onboarding/imessage.ts index 20c433ec4..7e89047e9 100644 --- a/src/channels/plugins/onboarding/imessage.ts +++ b/src/channels/plugins/onboarding/imessage.ts @@ -1,32 +1,51 @@ import { detectBinary } from "../../../commands/onboard-helpers.js"; import type { OpenClawConfig } from "../../../config/config.js"; -import type { DmPolicy } from "../../../config/types.js"; import { listIMessageAccountIds, resolveDefaultIMessageAccountId, resolveIMessageAccount, } from "../../../imessage/accounts.js"; import { normalizeIMessageHandle } from "../../../imessage/targets.js"; -import { DEFAULT_ACCOUNT_ID } from "../../../routing/session-key.js"; import { formatDocsLink } from "../../../terminal/links.js"; import type { WizardPrompter } from "../../../wizard/prompts.js"; import type { ChannelOnboardingAdapter, ChannelOnboardingDmPolicy } from "../onboarding-types.js"; import { - mergeAllowFromEntries, + parseOnboardingEntriesAllowingWildcard, + patchChannelConfigForAccount, + promptParsedAllowFromForScopedChannel, resolveAccountIdForConfigure, - resolveOnboardingAccountId, - setAccountAllowFromForChannel, setChannelDmPolicyWithAllowFrom, - splitOnboardingEntries, + setOnboardingChannelEnabled, } from "./helpers.js"; const channel = "imessage" as const; -function setIMessageDmPolicy(cfg: OpenClawConfig, dmPolicy: DmPolicy) { - return setChannelDmPolicyWithAllowFrom({ - cfg, - channel: "imessage", - dmPolicy, +export function parseIMessageAllowFromEntries(raw: string): { entries: string[]; error?: string } { + return parseOnboardingEntriesAllowingWildcard(raw, (entry) => { + const lower = entry.toLowerCase(); + if (lower.startsWith("chat_id:")) { + const id = entry.slice("chat_id:".length).trim(); + if (!/^\d+$/.test(id)) { + return { error: `Invalid chat_id: ${entry}` }; + } + return { value: entry }; + } + if (lower.startsWith("chat_guid:")) { + if (!entry.slice("chat_guid:".length).trim()) { + return { error: "Invalid chat_guid entry" }; + } + return { value: entry }; + } + if (lower.startsWith("chat_identifier:")) { + if (!entry.slice("chat_identifier:".length).trim()) { + return { error: "Invalid chat_identifier entry" }; + } + return { value: entry }; + } + if (!normalizeIMessageHandle(entry)) { + return { error: `Invalid handle: ${entry}` }; + } + return { value: entry }; }); } @@ -35,14 +54,14 @@ async function promptIMessageAllowFrom(params: { prompter: WizardPrompter; accountId?: string; }): Promise { - const accountId = resolveOnboardingAccountId({ + return promptParsedAllowFromForScopedChannel({ + cfg: params.cfg, + channel: "imessage", accountId: params.accountId, defaultAccountId: resolveDefaultIMessageAccountId(params.cfg), - }); - const resolved = resolveIMessageAccount({ cfg: params.cfg, accountId }); - const existing = resolved.config.allowFrom ?? []; - await params.prompter.note( - [ + prompter: params.prompter, + noteTitle: "iMessage allowlist", + noteLines: [ "Allowlist iMessage DMs by handle or chat target.", "Examples:", "- +15555550123", @@ -51,57 +70,15 @@ async function promptIMessageAllowFrom(params: { "- chat_guid:... or chat_identifier:...", "Multiple entries: comma-separated.", `Docs: ${formatDocsLink("/imessage", "imessage")}`, - ].join("\n"), - "iMessage allowlist", - ); - const entry = await params.prompter.text({ + ], message: "iMessage allowFrom (handle or chat_id)", placeholder: "+15555550123, user@example.com, chat_id:123", - initialValue: existing[0] ? String(existing[0]) : undefined, - validate: (value) => { - const raw = String(value ?? "").trim(); - if (!raw) { - return "Required"; - } - const parts = splitOnboardingEntries(raw); - for (const part of parts) { - if (part === "*") { - continue; - } - if (part.toLowerCase().startsWith("chat_id:")) { - const id = part.slice("chat_id:".length).trim(); - if (!/^\d+$/.test(id)) { - return `Invalid chat_id: ${part}`; - } - continue; - } - if (part.toLowerCase().startsWith("chat_guid:")) { - if (!part.slice("chat_guid:".length).trim()) { - return "Invalid chat_guid entry"; - } - continue; - } - if (part.toLowerCase().startsWith("chat_identifier:")) { - if (!part.slice("chat_identifier:".length).trim()) { - return "Invalid chat_identifier entry"; - } - continue; - } - if (!normalizeIMessageHandle(part)) { - return `Invalid handle: ${part}`; - } - } - return undefined; + parseEntries: parseIMessageAllowFromEntries, + getExistingAllowFrom: ({ cfg, accountId }) => { + const resolved = resolveIMessageAccount({ cfg, accountId }); + return resolved.config.allowFrom ?? []; }, }); - const parts = splitOnboardingEntries(String(entry)); - const unique = mergeAllowFromEntries(undefined, parts); - return setAccountAllowFromForChannel({ - cfg: params.cfg, - channel: "imessage", - accountId, - allowFrom: unique, - }); } const dmPolicy: ChannelOnboardingDmPolicy = { @@ -110,7 +87,12 @@ const dmPolicy: ChannelOnboardingDmPolicy = { policyKey: "channels.imessage.dmPolicy", allowFromKey: "channels.imessage.allowFrom", getCurrent: (cfg) => cfg.channels?.imessage?.dmPolicy ?? "pairing", - setPolicy: (cfg, policy) => setIMessageDmPolicy(cfg, policy), + setPolicy: (cfg, policy) => + setChannelDmPolicyWithAllowFrom({ + cfg, + channel: "imessage", + dmPolicy: policy, + }), promptAllowFrom: promptIMessageAllowFrom, }; @@ -172,38 +154,12 @@ export const imessageOnboardingAdapter: ChannelOnboardingAdapter = { } if (resolvedCliPath) { - if (imessageAccountId === DEFAULT_ACCOUNT_ID) { - next = { - ...next, - channels: { - ...next.channels, - imessage: { - ...next.channels?.imessage, - enabled: true, - cliPath: resolvedCliPath, - }, - }, - }; - } else { - next = { - ...next, - channels: { - ...next.channels, - imessage: { - ...next.channels?.imessage, - enabled: true, - accounts: { - ...next.channels?.imessage?.accounts, - [imessageAccountId]: { - ...next.channels?.imessage?.accounts?.[imessageAccountId], - enabled: next.channels?.imessage?.accounts?.[imessageAccountId]?.enabled ?? true, - cliPath: resolvedCliPath, - }, - }, - }, - }, - }; - } + next = patchChannelConfigForAccount({ + cfg: next, + channel: "imessage", + accountId: imessageAccountId, + patch: { cliPath: resolvedCliPath }, + }); } await prompter.note( @@ -220,11 +176,5 @@ export const imessageOnboardingAdapter: ChannelOnboardingAdapter = { return { cfg: next, accountId: imessageAccountId }; }, dmPolicy, - disable: (cfg) => ({ - ...cfg, - channels: { - ...cfg.channels, - imessage: { ...cfg.channels?.imessage, enabled: false }, - }, - }), + disable: (cfg) => setOnboardingChannelEnabled(cfg, channel, false), }; diff --git a/src/channels/plugins/onboarding/signal.test.ts b/src/channels/plugins/onboarding/signal.test.ts new file mode 100644 index 000000000..920b68f31 --- /dev/null +++ b/src/channels/plugins/onboarding/signal.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; +import { normalizeSignalAccountInput, parseSignalAllowFromEntries } from "./signal.js"; + +describe("normalizeSignalAccountInput", () => { + it("normalizes valid E.164 numbers", () => { + expect(normalizeSignalAccountInput(" +1 (555) 555-0123 ")).toBe("+15555550123"); + }); + + it("rejects invalid values", () => { + expect(normalizeSignalAccountInput("abc")).toBeNull(); + }); +}); + +describe("parseSignalAllowFromEntries", () => { + it("parses e164, uuid and wildcard entries", () => { + expect( + parseSignalAllowFromEntries("+15555550123, uuid:123e4567-e89b-12d3-a456-426614174000, *"), + ).toEqual({ + entries: ["+15555550123", "uuid:123e4567-e89b-12d3-a456-426614174000", "*"], + }); + }); + + it("normalizes bare uuid values", () => { + expect(parseSignalAllowFromEntries("123e4567-e89b-12d3-a456-426614174000")).toEqual({ + entries: ["uuid:123e4567-e89b-12d3-a456-426614174000"], + }); + }); + + it("returns validation errors for invalid entries", () => { + expect(parseSignalAllowFromEntries("uuid:")).toEqual({ + entries: [], + error: "Invalid uuid entry", + }); + expect(parseSignalAllowFromEntries("invalid")).toEqual({ + entries: [], + error: "Invalid entry: invalid", + }); + }); +}); diff --git a/src/channels/plugins/onboarding/signal.ts b/src/channels/plugins/onboarding/signal.ts index 4df479d86..ce48be2aa 100644 --- a/src/channels/plugins/onboarding/signal.ts +++ b/src/channels/plugins/onboarding/signal.ts @@ -2,8 +2,6 @@ import { formatCliCommand } from "../../../cli/command-format.js"; import { detectBinary } from "../../../commands/onboard-helpers.js"; import { installSignalCli } from "../../../commands/signal-install.js"; import type { OpenClawConfig } from "../../../config/config.js"; -import type { DmPolicy } from "../../../config/types.js"; -import { DEFAULT_ACCOUNT_ID } from "../../../routing/session-key.js"; import { listSignalAccountIds, resolveDefaultSignalAccountId, @@ -13,14 +11,7 @@ import { formatDocsLink } from "../../../terminal/links.js"; import { normalizeE164 } from "../../../utils.js"; import type { WizardPrompter } from "../../../wizard/prompts.js"; import type { ChannelOnboardingAdapter, ChannelOnboardingDmPolicy } from "../onboarding-types.js"; -import { - mergeAllowFromEntries, - resolveAccountIdForConfigure, - resolveOnboardingAccountId, - setAccountAllowFromForChannel, - setChannelDmPolicyWithAllowFrom, - splitOnboardingEntries, -} from "./helpers.js"; +import * as onboardingHelpers from "./helpers.js"; const channel = "signal" as const; const MIN_E164_DIGITS = 5; @@ -45,93 +36,58 @@ export function normalizeSignalAccountInput(value: string | null | undefined): s return `+${digits}`; } -function setSignalDmPolicy(cfg: OpenClawConfig, dmPolicy: DmPolicy) { - return setChannelDmPolicyWithAllowFrom({ - cfg, - channel: "signal", - dmPolicy, - }); -} - function isUuidLike(value: string): boolean { return /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(value); } +export function parseSignalAllowFromEntries(raw: string): { entries: string[]; error?: string } { + return onboardingHelpers.parseOnboardingEntriesAllowingWildcard(raw, (entry) => { + if (entry.toLowerCase().startsWith("uuid:")) { + const id = entry.slice("uuid:".length).trim(); + if (!id) { + return { error: "Invalid uuid entry" }; + } + return { value: `uuid:${id}` }; + } + if (isUuidLike(entry)) { + return { value: `uuid:${entry}` }; + } + const normalized = normalizeSignalAccountInput(entry); + if (!normalized) { + return { error: `Invalid entry: ${entry}` }; + } + return { value: normalized }; + }); +} + async function promptSignalAllowFrom(params: { cfg: OpenClawConfig; prompter: WizardPrompter; accountId?: string; }): Promise { - const accountId = resolveOnboardingAccountId({ + return onboardingHelpers.promptParsedAllowFromForScopedChannel({ + cfg: params.cfg, + channel: "signal", accountId: params.accountId, defaultAccountId: resolveDefaultSignalAccountId(params.cfg), - }); - const resolved = resolveSignalAccount({ cfg: params.cfg, accountId }); - const existing = resolved.config.allowFrom ?? []; - await params.prompter.note( - [ + prompter: params.prompter, + noteTitle: "Signal allowlist", + noteLines: [ "Allowlist Signal DMs by sender id.", "Examples:", "- +15555550123", "- uuid:123e4567-e89b-12d3-a456-426614174000", "Multiple entries: comma-separated.", `Docs: ${formatDocsLink("/signal", "signal")}`, - ].join("\n"), - "Signal allowlist", - ); - const entry = await params.prompter.text({ + ], message: "Signal allowFrom (E.164 or uuid)", placeholder: "+15555550123, uuid:123e4567-e89b-12d3-a456-426614174000", - initialValue: existing[0] ? String(existing[0]) : undefined, - validate: (value) => { - const raw = String(value ?? "").trim(); - if (!raw) { - return "Required"; - } - const parts = splitOnboardingEntries(raw); - for (const part of parts) { - if (part === "*") { - continue; - } - if (part.toLowerCase().startsWith("uuid:")) { - if (!part.slice("uuid:".length).trim()) { - return "Invalid uuid entry"; - } - continue; - } - if (isUuidLike(part)) { - continue; - } - if (!normalizeE164(part)) { - return `Invalid entry: ${part}`; - } - } - return undefined; + parseEntries: parseSignalAllowFromEntries, + getExistingAllowFrom: ({ cfg, accountId }) => { + const resolved = resolveSignalAccount({ cfg, accountId }); + return resolved.config.allowFrom ?? []; }, }); - const parts = splitOnboardingEntries(String(entry)); - const normalized = parts.map((part) => { - if (part === "*") { - return "*"; - } - if (part.toLowerCase().startsWith("uuid:")) { - return `uuid:${part.slice(5).trim()}`; - } - if (isUuidLike(part)) { - return `uuid:${part}`; - } - return normalizeE164(part); - }); - const unique = mergeAllowFromEntries( - undefined, - normalized.filter((part): part is string => typeof part === "string" && part.trim().length > 0), - ); - return setAccountAllowFromForChannel({ - cfg: params.cfg, - channel: "signal", - accountId, - allowFrom: unique, - }); } const dmPolicy: ChannelOnboardingDmPolicy = { @@ -140,7 +96,12 @@ const dmPolicy: ChannelOnboardingDmPolicy = { policyKey: "channels.signal.dmPolicy", allowFromKey: "channels.signal.allowFrom", getCurrent: (cfg) => cfg.channels?.signal?.dmPolicy ?? "pairing", - setPolicy: (cfg, policy) => setSignalDmPolicy(cfg, policy), + setPolicy: (cfg, policy) => + onboardingHelpers.setChannelDmPolicyWithAllowFrom({ + cfg, + channel: "signal", + dmPolicy: policy, + }), promptAllowFrom: promptSignalAllowFrom, }; @@ -172,7 +133,7 @@ export const signalOnboardingAdapter: ChannelOnboardingAdapter = { options, }) => { const defaultSignalAccountId = resolveDefaultSignalAccountId(cfg); - const signalAccountId = await resolveAccountIdForConfigure({ + const signalAccountId = await onboardingHelpers.resolveAccountIdForConfigure({ cfg, prompter, label: "Signal", @@ -255,40 +216,15 @@ export const signalOnboardingAdapter: ChannelOnboardingAdapter = { } if (account) { - if (signalAccountId === DEFAULT_ACCOUNT_ID) { - next = { - ...next, - channels: { - ...next.channels, - signal: { - ...next.channels?.signal, - enabled: true, - account, - cliPath: resolvedCliPath ?? "signal-cli", - }, - }, - }; - } else { - next = { - ...next, - channels: { - ...next.channels, - signal: { - ...next.channels?.signal, - enabled: true, - accounts: { - ...next.channels?.signal?.accounts, - [signalAccountId]: { - ...next.channels?.signal?.accounts?.[signalAccountId], - enabled: next.channels?.signal?.accounts?.[signalAccountId]?.enabled ?? true, - account, - cliPath: resolvedCliPath ?? "signal-cli", - }, - }, - }, - }, - }; - } + next = onboardingHelpers.patchChannelConfigForAccount({ + cfg: next, + channel: "signal", + accountId: signalAccountId, + patch: { + account, + cliPath: resolvedCliPath ?? "signal-cli", + }, + }); } await prompter.note( @@ -304,11 +240,5 @@ export const signalOnboardingAdapter: ChannelOnboardingAdapter = { return { cfg: next, accountId: signalAccountId }; }, dmPolicy, - disable: (cfg) => ({ - ...cfg, - channels: { - ...cfg.channels, - signal: { ...cfg.channels?.signal, enabled: false }, - }, - }), + disable: (cfg) => onboardingHelpers.setOnboardingChannelEnabled(cfg, channel, false), }; diff --git a/src/channels/plugins/onboarding/slack.ts b/src/channels/plugins/onboarding/slack.ts index 3937ce298..cd892bc0a 100644 --- a/src/channels/plugins/onboarding/slack.ts +++ b/src/channels/plugins/onboarding/slack.ts @@ -1,5 +1,4 @@ import type { OpenClawConfig } from "../../../config/config.js"; -import type { DmPolicy } from "../../../config/types.js"; import { DEFAULT_ACCOUNT_ID } from "../../../routing/session-key.js"; import { listSlackAccountIds, @@ -11,46 +10,22 @@ import { resolveSlackUserAllowlist } from "../../../slack/resolve-users.js"; import { formatDocsLink } from "../../../terminal/links.js"; import type { WizardPrompter } from "../../../wizard/prompts.js"; import type { ChannelOnboardingAdapter, ChannelOnboardingDmPolicy } from "../onboarding-types.js"; -import { promptChannelAccessConfig } from "./channel-access.js"; +import { configureChannelAccessWithAllowlist } from "./channel-access-configure.js"; import { - addWildcardAllowFrom, - promptResolvedAllowFrom, + parseMentionOrPrefixedId, + noteChannelLookupFailure, + noteChannelLookupSummary, + patchChannelConfigForAccount, + promptLegacyChannelAllowFrom, resolveAccountIdForConfigure, resolveOnboardingAccountId, - splitOnboardingEntries, + setAccountGroupPolicyForChannel, + setLegacyChannelDmPolicyWithAllowFrom, + setOnboardingChannelEnabled, } from "./helpers.js"; const channel = "slack" as const; -function patchSlackConfigWithDm( - cfg: OpenClawConfig, - patch: Record, -): OpenClawConfig { - return { - ...cfg, - channels: { - ...cfg.channels, - slack: { - ...cfg.channels?.slack, - ...patch, - dm: { - ...cfg.channels?.slack?.dm, - enabled: cfg.channels?.slack?.dm?.enabled ?? true, - }, - }, - }, - }; -} - -function setSlackDmPolicy(cfg: OpenClawConfig, dmPolicy: DmPolicy) { - const existingAllowFrom = cfg.channels?.slack?.allowFrom ?? cfg.channels?.slack?.dm?.allowFrom; - const allowFrom = dmPolicy === "open" ? addWildcardAllowFrom(existingAllowFrom) : undefined; - return patchSlackConfigWithDm(cfg, { - dmPolicy, - ...(allowFrom ? { allowFrom } : {}), - }); -} - function buildSlackManifest(botName: string) { const safeName = botName.trim() || "OpenClaw"; const manifest = { @@ -158,63 +133,18 @@ async function promptSlackTokens(prompter: WizardPrompter): Promise<{ return { botToken, appToken }; } -function patchSlackConfigForAccount( - cfg: OpenClawConfig, - accountId: string, - patch: Record, -): OpenClawConfig { - if (accountId === DEFAULT_ACCOUNT_ID) { - return { - ...cfg, - channels: { - ...cfg.channels, - slack: { - ...cfg.channels?.slack, - enabled: true, - ...patch, - }, - }, - }; - } - return { - ...cfg, - channels: { - ...cfg.channels, - slack: { - ...cfg.channels?.slack, - enabled: true, - accounts: { - ...cfg.channels?.slack?.accounts, - [accountId]: { - ...cfg.channels?.slack?.accounts?.[accountId], - enabled: cfg.channels?.slack?.accounts?.[accountId]?.enabled ?? true, - ...patch, - }, - }, - }, - }, - }; -} - -function setSlackGroupPolicy( - cfg: OpenClawConfig, - accountId: string, - groupPolicy: "open" | "allowlist" | "disabled", -): OpenClawConfig { - return patchSlackConfigForAccount(cfg, accountId, { groupPolicy }); -} - function setSlackChannelAllowlist( cfg: OpenClawConfig, accountId: string, channelKeys: string[], ): OpenClawConfig { const channels = Object.fromEntries(channelKeys.map((key) => [key, { allow: true }])); - return patchSlackConfigForAccount(cfg, accountId, { channels }); -} - -function setSlackAllowFrom(cfg: OpenClawConfig, allowFrom: string[]): OpenClawConfig { - return patchSlackConfigWithDm(cfg, { allowFrom }); + return patchChannelConfigForAccount({ + cfg, + channel: "slack", + accountId, + patch: { channels }, + }); } async function promptSlackAllowFrom(params: { @@ -230,42 +160,32 @@ async function promptSlackAllowFrom(params: { const token = resolved.config.userToken ?? resolved.config.botToken ?? ""; const existing = params.cfg.channels?.slack?.allowFrom ?? params.cfg.channels?.slack?.dm?.allowFrom ?? []; - await params.prompter.note( - [ + const parseId = (value: string) => + parseMentionOrPrefixedId({ + value, + mentionPattern: /^<@([A-Z0-9]+)>$/i, + prefixPattern: /^(slack:|user:)/i, + idPattern: /^[A-Z][A-Z0-9]+$/i, + normalizeId: (id) => id.toUpperCase(), + }); + + return promptLegacyChannelAllowFrom({ + cfg: params.cfg, + channel: "slack", + prompter: params.prompter, + existing, + token, + noteTitle: "Slack allowlist", + noteLines: [ "Allowlist Slack DMs by username (we resolve to user ids).", "Examples:", "- U12345678", "- @alice", "Multiple entries: comma-separated.", `Docs: ${formatDocsLink("/slack", "slack")}`, - ].join("\n"), - "Slack allowlist", - ); - const parseInputs = (value: string) => splitOnboardingEntries(value); - const parseId = (value: string) => { - const trimmed = value.trim(); - if (!trimmed) { - return null; - } - const mention = trimmed.match(/^<@([A-Z0-9]+)>$/i); - if (mention) { - return mention[1]?.toUpperCase(); - } - const prefixed = trimmed.replace(/^(slack:|user:)/i, ""); - if (/^[A-Z][A-Z0-9]+$/i.test(prefixed)) { - return prefixed.toUpperCase(); - } - return null; - }; - - const unique = await promptResolvedAllowFrom({ - prompter: params.prompter, - existing, - token, + ], message: "Slack allowFrom (usernames or ids)", placeholder: "@alice, U12345678", - label: "Slack allowlist", - parseInputs, parseId, invalidWithoutTokenNote: "Slack token missing; use user ids (or mention form) only.", resolveEntries: ({ token, entries }) => @@ -274,7 +194,6 @@ async function promptSlackAllowFrom(params: { entries, }), }); - return setSlackAllowFrom(params.cfg, unique); } const dmPolicy: ChannelOnboardingDmPolicy = { @@ -284,7 +203,12 @@ const dmPolicy: ChannelOnboardingDmPolicy = { allowFromKey: "channels.slack.allowFrom", getCurrent: (cfg) => cfg.channels?.slack?.dmPolicy ?? cfg.channels?.slack?.dm?.policy ?? "pairing", - setPolicy: (cfg, policy) => setSlackDmPolicy(cfg, policy), + setPolicy: (cfg, policy) => + setLegacyChannelDmPolicyWithAllowFrom({ + cfg, + channel: "slack", + dmPolicy: policy, + }), promptAllowFrom: promptSlackAllowFrom, }; @@ -347,13 +271,12 @@ export const slackOnboardingAdapter: ChannelOnboardingAdapter = { initialValue: true, }); if (keepEnv) { - next = { - ...next, - channels: { - ...next.channels, - slack: { ...next.channels?.slack, enabled: true }, - }, - }; + next = patchChannelConfigForAccount({ + cfg: next, + channel: "slack", + accountId: slackAccountId, + patch: {}, + }); } else { ({ botToken, appToken } = await promptSlackTokens(prompter)); } @@ -370,43 +293,16 @@ export const slackOnboardingAdapter: ChannelOnboardingAdapter = { } if (botToken && appToken) { - if (slackAccountId === DEFAULT_ACCOUNT_ID) { - next = { - ...next, - channels: { - ...next.channels, - slack: { - ...next.channels?.slack, - enabled: true, - botToken, - appToken, - }, - }, - }; - } else { - next = { - ...next, - channels: { - ...next.channels, - slack: { - ...next.channels?.slack, - enabled: true, - accounts: { - ...next.channels?.slack?.accounts, - [slackAccountId]: { - ...next.channels?.slack?.accounts?.[slackAccountId], - enabled: next.channels?.slack?.accounts?.[slackAccountId]?.enabled ?? true, - botToken, - appToken, - }, - }, - }, - }, - }; - } + next = patchChannelConfigForAccount({ + cfg: next, + channel: "slack", + accountId: slackAccountId, + patch: { botToken, appToken }, + }); } - const accessConfig = await promptChannelAccessConfig({ + next = await configureChannelAccessWithAllowlist({ + cfg: next, prompter, label: "Slack channels", currentPolicy: resolvedAccount.config.groupPolicy ?? "allowlist", @@ -415,21 +311,24 @@ export const slackOnboardingAdapter: ChannelOnboardingAdapter = { .map(([key]) => key), placeholder: "#general, #private, C123", updatePrompt: Boolean(resolvedAccount.config.channels), - }); - if (accessConfig) { - if (accessConfig.policy !== "allowlist") { - next = setSlackGroupPolicy(next, slackAccountId, accessConfig.policy); - } else { - let keys = accessConfig.entries; + setPolicy: (cfg, policy) => + setAccountGroupPolicyForChannel({ + cfg, + channel: "slack", + accountId: slackAccountId, + groupPolicy: policy, + }), + resolveAllowlist: async ({ cfg, entries }) => { + let keys = entries; const accountWithTokens = resolveSlackAccount({ - cfg: next, + cfg, accountId: slackAccountId, }); - if (accountWithTokens.botToken && accessConfig.entries.length > 0) { + if (accountWithTokens.botToken && entries.length > 0) { try { const resolved = await resolveSlackChannelAllowlist({ token: accountWithTokens.botToken, - entries: accessConfig.entries, + entries, }); const resolvedKeys = resolved .filter((entry) => entry.resolved && entry.id) @@ -438,39 +337,29 @@ export const slackOnboardingAdapter: ChannelOnboardingAdapter = { .filter((entry) => !entry.resolved) .map((entry) => entry.input); keys = [...resolvedKeys, ...unresolved.map((entry) => entry.trim()).filter(Boolean)]; - if (resolvedKeys.length > 0 || unresolved.length > 0) { - await prompter.note( - [ - resolvedKeys.length > 0 ? `Resolved: ${resolvedKeys.join(", ")}` : undefined, - unresolved.length > 0 - ? `Unresolved (kept as typed): ${unresolved.join(", ")}` - : undefined, - ] - .filter(Boolean) - .join("\n"), - "Slack channels", - ); - } + await noteChannelLookupSummary({ + prompter, + label: "Slack channels", + resolvedSections: [{ title: "Resolved", values: resolvedKeys }], + unresolved, + }); } catch (err) { - await prompter.note( - `Channel lookup failed; keeping entries as typed. ${String(err)}`, - "Slack channels", - ); + await noteChannelLookupFailure({ + prompter, + label: "Slack channels", + error: err, + }); } } - next = setSlackGroupPolicy(next, slackAccountId, "allowlist"); - next = setSlackChannelAllowlist(next, slackAccountId, keys); - } - } + return keys; + }, + applyAllowlist: ({ cfg, resolved }) => { + return setSlackChannelAllowlist(cfg, slackAccountId, resolved); + }, + }); return { cfg: next, accountId: slackAccountId }; }, dmPolicy, - disable: (cfg) => ({ - ...cfg, - channels: { - ...cfg.channels, - slack: { ...cfg.channels?.slack, enabled: false }, - }, - }), + disable: (cfg) => setOnboardingChannelEnabled(cfg, channel, false), }; diff --git a/src/channels/plugins/onboarding/telegram.test.ts b/src/channels/plugins/onboarding/telegram.test.ts new file mode 100644 index 000000000..98661ec99 --- /dev/null +++ b/src/channels/plugins/onboarding/telegram.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it } from "vitest"; +import { normalizeTelegramAllowFromInput, parseTelegramAllowFromId } from "./telegram.js"; + +describe("normalizeTelegramAllowFromInput", () => { + it("strips telegram/tg prefixes and trims whitespace", () => { + expect(normalizeTelegramAllowFromInput(" telegram:123 ")).toBe("123"); + expect(normalizeTelegramAllowFromInput("tg:@alice")).toBe("@alice"); + expect(normalizeTelegramAllowFromInput(" @bob ")).toBe("@bob"); + }); +}); + +describe("parseTelegramAllowFromId", () => { + it("accepts numeric ids with optional prefixes", () => { + expect(parseTelegramAllowFromId("12345")).toBe("12345"); + expect(parseTelegramAllowFromId("telegram:98765")).toBe("98765"); + expect(parseTelegramAllowFromId("tg:2468")).toBe("2468"); + }); + + it("rejects non-numeric values", () => { + expect(parseTelegramAllowFromId("@alice")).toBeNull(); + expect(parseTelegramAllowFromId("tg:alice")).toBeNull(); + }); +}); diff --git a/src/channels/plugins/onboarding/telegram.ts b/src/channels/plugins/onboarding/telegram.ts index 7efcaf914..10588268a 100644 --- a/src/channels/plugins/onboarding/telegram.ts +++ b/src/channels/plugins/onboarding/telegram.ts @@ -1,6 +1,5 @@ import { formatCliCommand } from "../../../cli/command-format.js"; import type { OpenClawConfig } from "../../../config/config.js"; -import type { DmPolicy } from "../../../config/types.js"; import { DEFAULT_ACCOUNT_ID } from "../../../routing/session-key.js"; import { listTelegramAccountIds, @@ -12,31 +11,19 @@ import type { WizardPrompter } from "../../../wizard/prompts.js"; import { fetchTelegramChatId } from "../../telegram/api.js"; import type { ChannelOnboardingAdapter, ChannelOnboardingDmPolicy } from "../onboarding-types.js"; import { - addWildcardAllowFrom, - mergeAllowFromEntries, + applySingleTokenPromptResult, + patchChannelConfigForAccount, + promptSingleChannelToken, + promptResolvedAllowFrom, resolveAccountIdForConfigure, resolveOnboardingAccountId, + setChannelDmPolicyWithAllowFrom, + setOnboardingChannelEnabled, splitOnboardingEntries, } from "./helpers.js"; const channel = "telegram" as const; -function setTelegramDmPolicy(cfg: OpenClawConfig, dmPolicy: DmPolicy) { - const allowFrom = - dmPolicy === "open" ? addWildcardAllowFrom(cfg.channels?.telegram?.allowFrom) : undefined; - return { - ...cfg, - channels: { - ...cfg.channels, - telegram: { - ...cfg.channels?.telegram, - dmPolicy, - ...(allowFrom ? { allowFrom } : {}), - }, - }, - }; -} - async function noteTelegramTokenHelp(prompter: WizardPrompter): Promise { await prompter.note( [ @@ -64,6 +51,18 @@ async function noteTelegramUserIdHelp(prompter: WizardPrompter): Promise { ); } +export function normalizeTelegramAllowFromInput(raw: string): string { + return raw + .trim() + .replace(/^(telegram|tg):/i, "") + .trim(); +} + +export function parseTelegramAllowFromId(raw: string): string | null { + const stripped = normalizeTelegramAllowFromInput(raw); + return /^\d+$/.test(stripped) ? stripped : null; +} + async function promptTelegramAllowFrom(params: { cfg: OpenClawConfig; prompter: WizardPrompter; @@ -78,80 +77,43 @@ async function promptTelegramAllowFrom(params: { if (!token) { await prompter.note("Telegram token missing; username lookup is unavailable.", "Telegram"); } - - const resolveTelegramUserId = async (raw: string): Promise => { - const trimmed = raw.trim(); - if (!trimmed) { - return null; - } - const stripped = trimmed.replace(/^(telegram|tg):/i, "").trim(); - if (/^\d+$/.test(stripped)) { - return stripped; - } - if (!token) { - return null; - } - const username = stripped.startsWith("@") ? stripped : `@${stripped}`; - return await fetchTelegramChatId({ token, chatId: username }); - }; - - let resolvedIds: string[] = []; - while (resolvedIds.length === 0) { - const entry = await prompter.text({ - message: "Telegram allowFrom (numeric sender id; @username resolves to id)", - placeholder: "@username", - initialValue: existingAllowFrom[0] ? String(existingAllowFrom[0]) : undefined, - validate: (value) => (String(value ?? "").trim() ? undefined : "Required"), - }); - const parts = splitOnboardingEntries(String(entry)); - const results = await Promise.all(parts.map((part) => resolveTelegramUserId(part))); - const unresolved = parts.filter((_, idx) => !results[idx]); - if (unresolved.length > 0) { - await prompter.note( - `Could not resolve: ${unresolved.join(", ")}. Use @username or numeric id.`, - "Telegram allowlist", + const unique = await promptResolvedAllowFrom({ + prompter, + existing: existingAllowFrom, + token, + message: "Telegram allowFrom (numeric sender id; @username resolves to id)", + placeholder: "@username", + label: "Telegram allowlist", + parseInputs: splitOnboardingEntries, + parseId: parseTelegramAllowFromId, + invalidWithoutTokenNote: + "Telegram token missing; use numeric sender ids (usernames require a bot token).", + resolveEntries: async ({ token: tokenValue, entries }) => { + const results = await Promise.all( + entries.map(async (entry) => { + const numericId = parseTelegramAllowFromId(entry); + if (numericId) { + return { input: entry, resolved: true, id: numericId }; + } + const stripped = normalizeTelegramAllowFromInput(entry); + if (!stripped) { + return { input: entry, resolved: false, id: null }; + } + const username = stripped.startsWith("@") ? stripped : `@${stripped}`; + const id = await fetchTelegramChatId({ token: tokenValue, chatId: username }); + return { input: entry, resolved: Boolean(id), id }; + }), ); - continue; - } - resolvedIds = results.filter(Boolean) as string[]; - } - - const unique = mergeAllowFromEntries(existingAllowFrom, resolvedIds); - - if (accountId === DEFAULT_ACCOUNT_ID) { - return { - ...cfg, - channels: { - ...cfg.channels, - telegram: { - ...cfg.channels?.telegram, - enabled: true, - dmPolicy: "allowlist", - allowFrom: unique, - }, - }, - }; - } - - return { - ...cfg, - channels: { - ...cfg.channels, - telegram: { - ...cfg.channels?.telegram, - enabled: true, - accounts: { - ...cfg.channels?.telegram?.accounts, - [accountId]: { - ...cfg.channels?.telegram?.accounts?.[accountId], - enabled: cfg.channels?.telegram?.accounts?.[accountId]?.enabled ?? true, - dmPolicy: "allowlist", - allowFrom: unique, - }, - }, - }, + return results; }, - }; + }); + + return patchChannelConfigForAccount({ + cfg, + channel: "telegram", + accountId, + patch: { dmPolicy: "allowlist", allowFrom: unique }, + }); } async function promptTelegramAllowFromForAccount(params: { @@ -176,7 +138,12 @@ const dmPolicy: ChannelOnboardingDmPolicy = { policyKey: "channels.telegram.dmPolicy", allowFromKey: "channels.telegram.allowFrom", getCurrent: (cfg) => cfg.channels?.telegram?.dmPolicy ?? "pairing", - setPolicy: (cfg, policy) => setTelegramDmPolicy(cfg, policy), + setPolicy: (cfg, policy) => + setChannelDmPolicyWithAllowFrom({ + cfg, + channel: "telegram", + dmPolicy: policy, + }), promptAllowFrom: promptTelegramAllowFromForAccount, }; @@ -219,95 +186,35 @@ export const telegramOnboardingAdapter: ChannelOnboardingAdapter = { }); const accountConfigured = Boolean(resolvedAccount.token); const allowEnv = telegramAccountId === DEFAULT_ACCOUNT_ID; - const canUseEnv = allowEnv && Boolean(process.env.TELEGRAM_BOT_TOKEN?.trim()); + const canUseEnv = + allowEnv && + !resolvedAccount.config.botToken && + Boolean(process.env.TELEGRAM_BOT_TOKEN?.trim()); const hasConfigToken = Boolean( resolvedAccount.config.botToken || resolvedAccount.config.tokenFile, ); - let token: string | null = null; if (!accountConfigured) { await noteTelegramTokenHelp(prompter); } - if (canUseEnv && !resolvedAccount.config.botToken) { - const keepEnv = await prompter.confirm({ - message: "TELEGRAM_BOT_TOKEN detected. Use env var?", - initialValue: true, - }); - if (keepEnv) { - next = { - ...next, - channels: { - ...next.channels, - telegram: { - ...next.channels?.telegram, - enabled: true, - }, - }, - }; - } else { - token = String( - await prompter.text({ - message: "Enter Telegram bot token", - validate: (value) => (value?.trim() ? undefined : "Required"), - }), - ).trim(); - } - } else if (hasConfigToken) { - const keep = await prompter.confirm({ - message: "Telegram token already configured. Keep it?", - initialValue: true, - }); - if (!keep) { - token = String( - await prompter.text({ - message: "Enter Telegram bot token", - validate: (value) => (value?.trim() ? undefined : "Required"), - }), - ).trim(); - } - } else { - token = String( - await prompter.text({ - message: "Enter Telegram bot token", - validate: (value) => (value?.trim() ? undefined : "Required"), - }), - ).trim(); - } - if (token) { - if (telegramAccountId === DEFAULT_ACCOUNT_ID) { - next = { - ...next, - channels: { - ...next.channels, - telegram: { - ...next.channels?.telegram, - enabled: true, - botToken: token, - }, - }, - }; - } else { - next = { - ...next, - channels: { - ...next.channels, - telegram: { - ...next.channels?.telegram, - enabled: true, - accounts: { - ...next.channels?.telegram?.accounts, - [telegramAccountId]: { - ...next.channels?.telegram?.accounts?.[telegramAccountId], - enabled: next.channels?.telegram?.accounts?.[telegramAccountId]?.enabled ?? true, - botToken: token, - }, - }, - }, - }, - }; - } - } + const tokenResult = await promptSingleChannelToken({ + prompter, + accountConfigured, + canUseEnv, + hasConfigToken, + envPrompt: "TELEGRAM_BOT_TOKEN detected. Use env var?", + keepPrompt: "Telegram token already configured. Keep it?", + inputPrompt: "Enter Telegram bot token", + }); + + next = applySingleTokenPromptResult({ + cfg: next, + channel: "telegram", + accountId: telegramAccountId, + tokenPatchKey: "botToken", + tokenResult, + }); if (forceAllowFrom) { next = await promptTelegramAllowFrom({ @@ -320,11 +227,5 @@ export const telegramOnboardingAdapter: ChannelOnboardingAdapter = { return { cfg: next, accountId: telegramAccountId }; }, dmPolicy, - disable: (cfg) => ({ - ...cfg, - channels: { - ...cfg.channels, - telegram: { ...cfg.channels?.telegram, enabled: false }, - }, - }), + disable: (cfg) => setOnboardingChannelEnabled(cfg, channel, false), }; diff --git a/src/channels/plugins/onboarding/whatsapp.test.ts b/src/channels/plugins/onboarding/whatsapp.test.ts index 90ba94060..369499bf0 100644 --- a/src/channels/plugins/onboarding/whatsapp.test.ts +++ b/src/channels/plugins/onboarding/whatsapp.test.ts @@ -81,6 +81,46 @@ function createRuntime(): RuntimeEnv { } as unknown as RuntimeEnv; } +async function runConfigureWithHarness(params: { + harness: ReturnType; + cfg?: Parameters[0]["cfg"]; + runtime?: RuntimeEnv; + options?: Parameters[0]["options"]; + accountOverrides?: Parameters[0]["accountOverrides"]; + shouldPromptAccountIds?: boolean; + forceAllowFrom?: boolean; +}) { + return await whatsappOnboardingAdapter.configure({ + cfg: params.cfg ?? {}, + runtime: params.runtime ?? createRuntime(), + prompter: params.harness.prompter, + options: params.options ?? {}, + accountOverrides: params.accountOverrides ?? {}, + shouldPromptAccountIds: params.shouldPromptAccountIds ?? false, + forceAllowFrom: params.forceAllowFrom ?? false, + }); +} + +function createSeparatePhoneHarness(params: { selectValues: string[]; textValues?: string[] }) { + return createPrompterHarness({ + confirmValues: [false], + selectValues: params.selectValues, + textValues: params.textValues, + }); +} + +async function runSeparatePhoneFlow(params: { selectValues: string[]; textValues?: string[] }) { + pathExistsMock.mockResolvedValue(true); + const harness = createSeparatePhoneHarness({ + selectValues: params.selectValues, + textValues: params.textValues, + }); + const result = await runConfigureWithHarness({ + harness, + }); + return { harness, result }; +} + describe("whatsappOnboardingAdapter.configure", () => { beforeEach(() => { vi.clearAllMocks(); @@ -96,13 +136,8 @@ describe("whatsappOnboardingAdapter.configure", () => { textValues: ["+1 (555) 555-0123"], }); - const result = await whatsappOnboardingAdapter.configure({ - cfg: {}, - runtime: createRuntime(), - prompter: harness.prompter, - options: {}, - accountOverrides: {}, - shouldPromptAccountIds: false, + const result = await runConfigureWithHarness({ + harness, forceAllowFrom: true, }); @@ -119,22 +154,10 @@ describe("whatsappOnboardingAdapter.configure", () => { }); it("supports disabled DM policy for separate-phone setup", async () => { - pathExistsMock.mockResolvedValue(true); - const harness = createPrompterHarness({ - confirmValues: [false], + const { harness, result } = await runSeparatePhoneFlow({ selectValues: ["separate", "disabled"], }); - const result = await whatsappOnboardingAdapter.configure({ - cfg: {}, - runtime: createRuntime(), - prompter: harness.prompter, - options: {}, - accountOverrides: {}, - shouldPromptAccountIds: false, - forceAllowFrom: false, - }); - expect(result.cfg.channels?.whatsapp?.selfChatMode).toBe(false); expect(result.cfg.channels?.whatsapp?.dmPolicy).toBe("disabled"); expect(result.cfg.channels?.whatsapp?.allowFrom).toBeUndefined(); @@ -142,23 +165,11 @@ describe("whatsappOnboardingAdapter.configure", () => { }); it("normalizes allowFrom entries when list mode is selected", async () => { - pathExistsMock.mockResolvedValue(true); - const harness = createPrompterHarness({ - confirmValues: [false], + const { result } = await runSeparatePhoneFlow({ selectValues: ["separate", "allowlist", "list"], textValues: ["+1 (555) 555-0123, +15555550123, *"], }); - const result = await whatsappOnboardingAdapter.configure({ - cfg: {}, - runtime: createRuntime(), - prompter: harness.prompter, - options: {}, - accountOverrides: {}, - shouldPromptAccountIds: false, - forceAllowFrom: false, - }); - expect(result.cfg.channels?.whatsapp?.selfChatMode).toBe(false); expect(result.cfg.channels?.whatsapp?.dmPolicy).toBe("allowlist"); expect(result.cfg.channels?.whatsapp?.allowFrom).toEqual(["+15555550123", "*"]); @@ -172,14 +183,8 @@ describe("whatsappOnboardingAdapter.configure", () => { textValues: ["+1 (555) 111-2222"], }); - const result = await whatsappOnboardingAdapter.configure({ - cfg: {}, - runtime: createRuntime(), - prompter: harness.prompter, - options: {}, - accountOverrides: {}, - shouldPromptAccountIds: false, - forceAllowFrom: false, + const result = await runConfigureWithHarness({ + harness, }); expect(result.cfg.channels?.whatsapp?.selfChatMode).toBe(true); @@ -189,12 +194,12 @@ describe("whatsappOnboardingAdapter.configure", () => { it("forces wildcard allowFrom for open policy without allowFrom follow-up prompts", async () => { pathExistsMock.mockResolvedValue(true); - const harness = createPrompterHarness({ - confirmValues: [false], + const harness = createSeparatePhoneHarness({ selectValues: ["separate", "open"], }); - const result = await whatsappOnboardingAdapter.configure({ + const result = await runConfigureWithHarness({ + harness, cfg: { channels: { whatsapp: { @@ -202,12 +207,6 @@ describe("whatsappOnboardingAdapter.configure", () => { }, }, }, - runtime: createRuntime(), - prompter: harness.prompter, - options: {}, - accountOverrides: {}, - shouldPromptAccountIds: false, - forceAllowFrom: false, }); expect(result.cfg.channels?.whatsapp?.selfChatMode).toBe(false); @@ -225,14 +224,9 @@ describe("whatsappOnboardingAdapter.configure", () => { }); const runtime = createRuntime(); - await whatsappOnboardingAdapter.configure({ - cfg: {}, + await runConfigureWithHarness({ + harness, runtime, - prompter: harness.prompter, - options: {}, - accountOverrides: {}, - shouldPromptAccountIds: false, - forceAllowFrom: false, }); expect(loginWebMock).toHaveBeenCalledWith(false, undefined, runtime, DEFAULT_ACCOUNT_ID); @@ -240,19 +234,12 @@ describe("whatsappOnboardingAdapter.configure", () => { it("skips relink note when already linked and relink is declined", async () => { pathExistsMock.mockResolvedValue(true); - const harness = createPrompterHarness({ - confirmValues: [false], + const harness = createSeparatePhoneHarness({ selectValues: ["separate", "disabled"], }); - await whatsappOnboardingAdapter.configure({ - cfg: {}, - runtime: createRuntime(), - prompter: harness.prompter, - options: {}, - accountOverrides: {}, - shouldPromptAccountIds: false, - forceAllowFrom: false, + await runConfigureWithHarness({ + harness, }); expect(loginWebMock).not.toHaveBeenCalled(); @@ -264,19 +251,12 @@ describe("whatsappOnboardingAdapter.configure", () => { it("shows follow-up login command note when not linked and linking is skipped", async () => { pathExistsMock.mockResolvedValue(false); - const harness = createPrompterHarness({ - confirmValues: [false], + const harness = createSeparatePhoneHarness({ selectValues: ["separate", "disabled"], }); - await whatsappOnboardingAdapter.configure({ - cfg: {}, - runtime: createRuntime(), - prompter: harness.prompter, - options: {}, - accountOverrides: {}, - shouldPromptAccountIds: false, - forceAllowFrom: false, + await runConfigureWithHarness({ + harness, }); expect(harness.note).toHaveBeenCalledWith( diff --git a/src/channels/plugins/outbound/direct-text-media.ts b/src/channels/plugins/outbound/direct-text-media.ts new file mode 100644 index 000000000..02b97078d --- /dev/null +++ b/src/channels/plugins/outbound/direct-text-media.ts @@ -0,0 +1,119 @@ +import { chunkText } from "../../../auto-reply/chunk.js"; +import type { OpenClawConfig } from "../../../config/config.js"; +import type { OutboundSendDeps } from "../../../infra/outbound/deliver.js"; +import { resolveChannelMediaMaxBytes } from "../media-limits.js"; +import type { ChannelOutboundAdapter } from "../types.js"; + +type DirectSendOptions = { + accountId?: string | null; + replyToId?: string | null; + mediaUrl?: string; + mediaLocalRoots?: readonly string[]; + maxBytes?: number; +}; + +type DirectSendResult = { messageId: string; [key: string]: unknown }; + +type DirectSendFn, TResult extends DirectSendResult> = ( + to: string, + text: string, + opts: TOpts, +) => Promise; + +export function resolveScopedChannelMediaMaxBytes(params: { + cfg: OpenClawConfig; + accountId?: string | null; + resolveChannelLimitMb: (params: { cfg: OpenClawConfig; accountId: string }) => number | undefined; +}): number | undefined { + return resolveChannelMediaMaxBytes({ + cfg: params.cfg, + resolveChannelLimitMb: params.resolveChannelLimitMb, + accountId: params.accountId, + }); +} + +export function createScopedChannelMediaMaxBytesResolver(channel: "imessage" | "signal") { + return (params: { cfg: OpenClawConfig; accountId?: string | null }) => + resolveScopedChannelMediaMaxBytes({ + cfg: params.cfg, + accountId: params.accountId, + resolveChannelLimitMb: ({ cfg, accountId }) => + cfg.channels?.[channel]?.accounts?.[accountId]?.mediaMaxMb ?? + cfg.channels?.[channel]?.mediaMaxMb, + }); +} + +export function createDirectTextMediaOutbound< + TOpts extends Record, + TResult extends DirectSendResult, +>(params: { + channel: "imessage" | "signal"; + resolveSender: (deps: OutboundSendDeps | undefined) => DirectSendFn; + resolveMaxBytes: (params: { + cfg: OpenClawConfig; + accountId?: string | null; + }) => number | undefined; + buildTextOptions: (params: DirectSendOptions) => TOpts; + buildMediaOptions: (params: DirectSendOptions) => TOpts; +}): ChannelOutboundAdapter { + const sendDirect = async (sendParams: { + cfg: OpenClawConfig; + to: string; + text: string; + accountId?: string | null; + deps?: OutboundSendDeps; + replyToId?: string | null; + mediaUrl?: string; + mediaLocalRoots?: readonly string[]; + buildOptions: (params: DirectSendOptions) => TOpts; + }) => { + const send = params.resolveSender(sendParams.deps); + const maxBytes = params.resolveMaxBytes({ + cfg: sendParams.cfg, + accountId: sendParams.accountId, + }); + const result = await send( + sendParams.to, + sendParams.text, + sendParams.buildOptions({ + mediaUrl: sendParams.mediaUrl, + mediaLocalRoots: sendParams.mediaLocalRoots, + accountId: sendParams.accountId, + replyToId: sendParams.replyToId, + maxBytes, + }), + ); + return { channel: params.channel, ...result }; + }; + + return { + deliveryMode: "direct", + chunker: chunkText, + chunkerMode: "text", + textChunkLimit: 4000, + sendText: async ({ cfg, to, text, accountId, deps, replyToId }) => { + return await sendDirect({ + cfg, + to, + text, + accountId, + deps, + replyToId, + buildOptions: params.buildTextOptions, + }); + }, + sendMedia: async ({ cfg, to, text, mediaUrl, mediaLocalRoots, accountId, deps, replyToId }) => { + return await sendDirect({ + cfg, + to, + text, + mediaUrl, + mediaLocalRoots, + accountId, + deps, + replyToId, + buildOptions: params.buildMediaOptions, + }); + }, + }; +} diff --git a/src/channels/plugins/outbound/discord.test.ts b/src/channels/plugins/outbound/discord.test.ts index 1d14a9271..70e74da0d 100644 --- a/src/channels/plugins/outbound/discord.test.ts +++ b/src/channels/plugins/outbound/discord.test.ts @@ -36,6 +36,28 @@ vi.mock("../../../discord/monitor/thread-bindings.js", async (importOriginal) => const { discordOutbound } = await import("./discord.js"); +const DEFAULT_DISCORD_SEND_RESULT = { + channel: "discord", + messageId: "msg-1", + channelId: "ch-1", +} as const; + +function expectThreadBotSend(params: { + text: string; + result: unknown; + options?: Record; +}) { + expect(hoisted.sendMessageDiscordMock).toHaveBeenCalledWith( + "channel:thread-1", + params.text, + expect.objectContaining({ + accountId: "default", + ...params.options, + }), + ); + expect(params.result).toEqual(DEFAULT_DISCORD_SEND_RESULT); +} + function mockBoundThreadManager() { hoisted.getThreadBindingManagerMock.mockReturnValue({ getByThreadId: () => ({ @@ -113,17 +135,9 @@ describe("discordOutbound", () => { threadId: "thread-1", }); - expect(hoisted.sendMessageDiscordMock).toHaveBeenCalledWith( - "channel:thread-1", - "hello", - expect.objectContaining({ - accountId: "default", - }), - ); - expect(result).toEqual({ - channel: "discord", - messageId: "msg-1", - channelId: "ch-1", + expectThreadBotSend({ + text: "hello", + result, }); }); @@ -176,18 +190,10 @@ describe("discordOutbound", () => { }); expect(hoisted.sendWebhookMessageDiscordMock).not.toHaveBeenCalled(); - expect(hoisted.sendMessageDiscordMock).toHaveBeenCalledWith( - "channel:thread-1", - "silent update", - expect.objectContaining({ - accountId: "default", - silent: true, - }), - ); - expect(result).toEqual({ - channel: "discord", - messageId: "msg-1", - channelId: "ch-1", + expectThreadBotSend({ + text: "silent update", + result, + options: { silent: true }, }); }); @@ -204,17 +210,9 @@ describe("discordOutbound", () => { }); expect(hoisted.sendWebhookMessageDiscordMock).toHaveBeenCalledTimes(1); - expect(hoisted.sendMessageDiscordMock).toHaveBeenCalledWith( - "channel:thread-1", - "fallback", - expect.objectContaining({ - accountId: "default", - }), - ); - expect(result).toEqual({ - channel: "discord", - messageId: "msg-1", - channelId: "ch-1", + expectThreadBotSend({ + text: "fallback", + result, }); }); diff --git a/src/channels/plugins/outbound/imessage.ts b/src/channels/plugins/outbound/imessage.ts index 6888ef1d5..6a419bc27 100644 --- a/src/channels/plugins/outbound/imessage.ts +++ b/src/channels/plugins/outbound/imessage.ts @@ -1,46 +1,28 @@ -import { chunkText } from "../../../auto-reply/chunk.js"; import { sendMessageIMessage } from "../../../imessage/send.js"; -import { resolveChannelMediaMaxBytes } from "../media-limits.js"; -import type { ChannelOutboundAdapter } from "../types.js"; +import type { OutboundSendDeps } from "../../../infra/outbound/deliver.js"; +import { + createScopedChannelMediaMaxBytesResolver, + createDirectTextMediaOutbound, +} from "./direct-text-media.js"; -function resolveIMessageMaxBytes(params: { - cfg: Parameters[0]["cfg"]; - accountId?: string | null; -}) { - return resolveChannelMediaMaxBytes({ - cfg: params.cfg, - resolveChannelLimitMb: ({ cfg, accountId }) => - cfg.channels?.imessage?.accounts?.[accountId]?.mediaMaxMb ?? - cfg.channels?.imessage?.mediaMaxMb, - accountId: params.accountId, - }); +function resolveIMessageSender(deps: OutboundSendDeps | undefined) { + return deps?.sendIMessage ?? sendMessageIMessage; } -export const imessageOutbound: ChannelOutboundAdapter = { - deliveryMode: "direct", - chunker: chunkText, - chunkerMode: "text", - textChunkLimit: 4000, - sendText: async ({ cfg, to, text, accountId, deps, replyToId }) => { - const send = deps?.sendIMessage ?? sendMessageIMessage; - const maxBytes = resolveIMessageMaxBytes({ cfg, accountId }); - const result = await send(to, text, { - maxBytes, - accountId: accountId ?? undefined, - replyToId: replyToId ?? undefined, - }); - return { channel: "imessage", ...result }; - }, - sendMedia: async ({ cfg, to, text, mediaUrl, mediaLocalRoots, accountId, deps, replyToId }) => { - const send = deps?.sendIMessage ?? sendMessageIMessage; - const maxBytes = resolveIMessageMaxBytes({ cfg, accountId }); - const result = await send(to, text, { - mediaUrl, - maxBytes, - accountId: accountId ?? undefined, - replyToId: replyToId ?? undefined, - mediaLocalRoots, - }); - return { channel: "imessage", ...result }; - }, -}; +export const imessageOutbound = createDirectTextMediaOutbound({ + channel: "imessage", + resolveSender: resolveIMessageSender, + resolveMaxBytes: createScopedChannelMediaMaxBytesResolver("imessage"), + buildTextOptions: ({ maxBytes, accountId, replyToId }) => ({ + maxBytes, + accountId: accountId ?? undefined, + replyToId: replyToId ?? undefined, + }), + buildMediaOptions: ({ mediaUrl, maxBytes, accountId, replyToId, mediaLocalRoots }) => ({ + mediaUrl, + maxBytes, + accountId: accountId ?? undefined, + replyToId: replyToId ?? undefined, + mediaLocalRoots, + }), +}); diff --git a/src/channels/plugins/outbound/load.ts b/src/channels/plugins/outbound/load.ts index 8f027b373..131924e70 100644 --- a/src/channels/plugins/outbound/load.ts +++ b/src/channels/plugins/outbound/load.ts @@ -1,5 +1,4 @@ -import type { PluginRegistry } from "../../../plugins/registry.js"; -import { getActivePluginRegistry } from "../../../plugins/runtime.js"; +import { createChannelRegistryLoader } from "../registry-loader.js"; import type { ChannelId, ChannelOutboundAdapter } from "../types.js"; // Channel docking: outbound sends should stay cheap to import. @@ -7,31 +6,12 @@ import type { ChannelId, ChannelOutboundAdapter } from "../types.js"; // The full channel plugins (src/channels/plugins/*.ts) pull in status, // onboarding, gateway monitors, etc. Outbound delivery only needs chunking + // send primitives, so we keep a dedicated, lightweight loader here. -const cache = new Map(); -let lastRegistry: PluginRegistry | null = null; - -function ensureCacheForRegistry(registry: PluginRegistry | null) { - if (registry === lastRegistry) { - return; - } - cache.clear(); - lastRegistry = registry; -} +const loadOutboundAdapterFromRegistry = createChannelRegistryLoader( + (entry) => entry.plugin.outbound, +); export async function loadChannelOutboundAdapter( id: ChannelId, ): Promise { - const registry = getActivePluginRegistry(); - ensureCacheForRegistry(registry); - const cached = cache.get(id); - if (cached) { - return cached; - } - const pluginEntry = registry?.channels.find((entry) => entry.plugin.id === id); - const outbound = pluginEntry?.plugin.outbound; - if (outbound) { - cache.set(id, outbound); - return outbound; - } - return undefined; + return loadOutboundAdapterFromRegistry(id); } diff --git a/src/channels/plugins/outbound/signal.test.ts b/src/channels/plugins/outbound/signal.test.ts new file mode 100644 index 000000000..6d1d0bd06 --- /dev/null +++ b/src/channels/plugins/outbound/signal.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../../../config/config.js"; +import { signalOutbound } from "./signal.js"; + +describe("signalOutbound", () => { + const cfg: OpenClawConfig = { + channels: { + signal: { + mediaMaxMb: 8, + accounts: { + work: { + mediaMaxMb: 4, + }, + }, + }, + }, + }; + + it("passes account-scoped maxBytes for sendText", async () => { + const sendSignal = vi.fn().mockResolvedValue({ messageId: "sig-text-1", timestamp: 123 }); + const sendText = signalOutbound.sendText; + expect(sendText).toBeDefined(); + + const result = await sendText!({ + cfg, + to: "+15555550123", + text: "hello", + accountId: "work", + deps: { sendSignal }, + }); + + expect(sendSignal).toHaveBeenCalledWith( + "+15555550123", + "hello", + expect.objectContaining({ + accountId: "work", + maxBytes: 4 * 1024 * 1024, + }), + ); + expect(result).toEqual({ channel: "signal", messageId: "sig-text-1", timestamp: 123 }); + }); + + it("passes mediaUrl/mediaLocalRoots for sendMedia", async () => { + const sendSignal = vi.fn().mockResolvedValue({ messageId: "sig-media-1", timestamp: 456 }); + const sendMedia = signalOutbound.sendMedia; + expect(sendMedia).toBeDefined(); + + const result = await sendMedia!({ + cfg, + to: "+15555550124", + text: "caption", + mediaUrl: "https://example.com/file.jpg", + mediaLocalRoots: ["/tmp/media"], + accountId: "default", + deps: { sendSignal }, + }); + + expect(sendSignal).toHaveBeenCalledWith( + "+15555550124", + "caption", + expect.objectContaining({ + mediaUrl: "https://example.com/file.jpg", + mediaLocalRoots: ["/tmp/media"], + accountId: "default", + maxBytes: 8 * 1024 * 1024, + }), + ); + expect(result).toEqual({ channel: "signal", messageId: "sig-media-1", timestamp: 456 }); + }); +}); diff --git a/src/channels/plugins/outbound/signal.ts b/src/channels/plugins/outbound/signal.ts index cad9a13ef..e91feacad 100644 --- a/src/channels/plugins/outbound/signal.ts +++ b/src/channels/plugins/outbound/signal.ts @@ -1,43 +1,26 @@ -import { chunkText } from "../../../auto-reply/chunk.js"; +import type { OutboundSendDeps } from "../../../infra/outbound/deliver.js"; import { sendMessageSignal } from "../../../signal/send.js"; -import { resolveChannelMediaMaxBytes } from "../media-limits.js"; -import type { ChannelOutboundAdapter } from "../types.js"; +import { + createScopedChannelMediaMaxBytesResolver, + createDirectTextMediaOutbound, +} from "./direct-text-media.js"; -function resolveSignalMaxBytes(params: { - cfg: Parameters[0]["cfg"]; - accountId?: string | null; -}) { - return resolveChannelMediaMaxBytes({ - cfg: params.cfg, - resolveChannelLimitMb: ({ cfg, accountId }) => - cfg.channels?.signal?.accounts?.[accountId]?.mediaMaxMb ?? cfg.channels?.signal?.mediaMaxMb, - accountId: params.accountId, - }); +function resolveSignalSender(deps: OutboundSendDeps | undefined) { + return deps?.sendSignal ?? sendMessageSignal; } -export const signalOutbound: ChannelOutboundAdapter = { - deliveryMode: "direct", - chunker: chunkText, - chunkerMode: "text", - textChunkLimit: 4000, - sendText: async ({ cfg, to, text, accountId, deps }) => { - const send = deps?.sendSignal ?? sendMessageSignal; - const maxBytes = resolveSignalMaxBytes({ cfg, accountId }); - const result = await send(to, text, { - maxBytes, - accountId: accountId ?? undefined, - }); - return { channel: "signal", ...result }; - }, - sendMedia: async ({ cfg, to, text, mediaUrl, mediaLocalRoots, accountId, deps }) => { - const send = deps?.sendSignal ?? sendMessageSignal; - const maxBytes = resolveSignalMaxBytes({ cfg, accountId }); - const result = await send(to, text, { - mediaUrl, - maxBytes, - accountId: accountId ?? undefined, - mediaLocalRoots, - }); - return { channel: "signal", ...result }; - }, -}; +export const signalOutbound = createDirectTextMediaOutbound({ + channel: "signal", + resolveSender: resolveSignalSender, + resolveMaxBytes: createScopedChannelMediaMaxBytesResolver("signal"), + buildTextOptions: ({ maxBytes, accountId }) => ({ + maxBytes, + accountId: accountId ?? undefined, + }), + buildMediaOptions: ({ mediaUrl, maxBytes, accountId, mediaLocalRoots }) => ({ + mediaUrl, + maxBytes, + accountId: accountId ?? undefined, + mediaLocalRoots, + }), +}); diff --git a/src/channels/plugins/outbound/slack.test.ts b/src/channels/plugins/outbound/slack.test.ts index 0c009d461..42583a25b 100644 --- a/src/channels/plugins/outbound/slack.test.ts +++ b/src/channels/plugins/outbound/slack.test.ts @@ -13,7 +13,7 @@ import { getGlobalHookRunner } from "../../../plugins/hook-runner-global.js"; import { sendMessageSlack } from "../../../slack/send.js"; import { slackOutbound } from "./slack.js"; -const sendSlackText = async (ctx: { +type SlackSendTextCtx = { to: string; text: string; accountId: string; @@ -23,7 +23,15 @@ const sendSlackText = async (ctx: { avatarUrl?: string; emoji?: string; }; -}) => { +}; + +const BASE_SLACK_SEND_CTX = { + to: "C123", + accountId: "default", + replyToId: "1111.2222", +} as const; + +const sendSlackText = async (ctx: SlackSendTextCtx) => { const sendText = slackOutbound.sendText as NonNullable; return await sendText({ cfg: {} as OpenClawConfig, @@ -31,6 +39,32 @@ const sendSlackText = async (ctx: { }); }; +const sendSlackTextWithDefaults = async ( + overrides: Partial & Pick, +) => { + return await sendSlackText({ + ...BASE_SLACK_SEND_CTX, + ...overrides, + }); +}; + +const expectSlackSendCalledWith = ( + text: string, + options?: { + identity?: { + username?: string; + iconUrl?: string; + iconEmoji?: string; + }; + }, +) => { + expect(sendMessageSlack).toHaveBeenCalledWith("C123", text, { + threadTs: "1111.2222", + accountId: "default", + ...options, + }); +}; + describe("slack outbound hook wiring", () => { beforeEach(() => { vi.clearAllMocks(); @@ -43,27 +77,15 @@ describe("slack outbound hook wiring", () => { it("calls send without hooks when no hooks registered", async () => { vi.mocked(getGlobalHookRunner).mockReturnValue(null); - await sendSlackText({ - to: "C123", - text: "hello", - accountId: "default", - replyToId: "1111.2222", - }); - - expect(sendMessageSlack).toHaveBeenCalledWith("C123", "hello", { - threadTs: "1111.2222", - accountId: "default", - }); + await sendSlackTextWithDefaults({ text: "hello" }); + expectSlackSendCalledWith("hello"); }); it("forwards identity opts when present", async () => { vi.mocked(getGlobalHookRunner).mockReturnValue(null); - await sendSlackText({ - to: "C123", + await sendSlackTextWithDefaults({ text: "hello", - accountId: "default", - replyToId: "1111.2222", identity: { name: "My Agent", avatarUrl: "https://example.com/avatar.png", @@ -71,9 +93,7 @@ describe("slack outbound hook wiring", () => { }, }); - expect(sendMessageSlack).toHaveBeenCalledWith("C123", "hello", { - threadTs: "1111.2222", - accountId: "default", + expectSlackSendCalledWith("hello", { identity: { username: "My Agent", iconUrl: "https://example.com/avatar.png" }, }); }); @@ -81,17 +101,12 @@ describe("slack outbound hook wiring", () => { it("forwards icon_emoji only when icon_url is absent", async () => { vi.mocked(getGlobalHookRunner).mockReturnValue(null); - await sendSlackText({ - to: "C123", + await sendSlackTextWithDefaults({ text: "hello", - accountId: "default", - replyToId: "1111.2222", identity: { emoji: ":lobster:" }, }); - expect(sendMessageSlack).toHaveBeenCalledWith("C123", "hello", { - threadTs: "1111.2222", - accountId: "default", + expectSlackSendCalledWith("hello", { identity: { iconEmoji: ":lobster:" }, }); }); @@ -104,22 +119,14 @@ describe("slack outbound hook wiring", () => { // oxlint-disable-next-line typescript/no-explicit-any vi.mocked(getGlobalHookRunner).mockReturnValue(mockRunner as any); - await sendSlackText({ - to: "C123", - text: "hello", - accountId: "default", - replyToId: "1111.2222", - }); + await sendSlackTextWithDefaults({ text: "hello" }); expect(mockRunner.hasHooks).toHaveBeenCalledWith("message_sending"); expect(mockRunner.runMessageSending).toHaveBeenCalledWith( { to: "C123", content: "hello", metadata: { threadTs: "1111.2222", channelId: "C123" } }, { channelId: "slack", accountId: "default" }, ); - expect(sendMessageSlack).toHaveBeenCalledWith("C123", "hello", { - threadTs: "1111.2222", - accountId: "default", - }); + expectSlackSendCalledWith("hello"); }); it("cancels send when hook returns cancel:true", async () => { @@ -130,12 +137,7 @@ describe("slack outbound hook wiring", () => { // oxlint-disable-next-line typescript/no-explicit-any vi.mocked(getGlobalHookRunner).mockReturnValue(mockRunner as any); - const result = await sendSlackText({ - to: "C123", - text: "hello", - accountId: "default", - replyToId: "1111.2222", - }); + const result = await sendSlackTextWithDefaults({ text: "hello" }); expect(sendMessageSlack).not.toHaveBeenCalled(); expect(result.channel).toBe("slack"); @@ -149,17 +151,8 @@ describe("slack outbound hook wiring", () => { // oxlint-disable-next-line typescript/no-explicit-any vi.mocked(getGlobalHookRunner).mockReturnValue(mockRunner as any); - await sendSlackText({ - to: "C123", - text: "original", - accountId: "default", - replyToId: "1111.2222", - }); - - expect(sendMessageSlack).toHaveBeenCalledWith("C123", "modified", { - threadTs: "1111.2222", - accountId: "default", - }); + await sendSlackTextWithDefaults({ text: "original" }); + expectSlackSendCalledWith("modified"); }); it("skips hooks when runner has no message_sending hooks", async () => { @@ -170,12 +163,7 @@ describe("slack outbound hook wiring", () => { // oxlint-disable-next-line typescript/no-explicit-any vi.mocked(getGlobalHookRunner).mockReturnValue(mockRunner as any); - await sendSlackText({ - to: "C123", - text: "hello", - accountId: "default", - replyToId: "1111.2222", - }); + await sendSlackTextWithDefaults({ text: "hello" }); expect(mockRunner.runMessageSending).not.toHaveBeenCalled(); expect(sendMessageSlack).toHaveBeenCalled(); diff --git a/src/channels/plugins/outbound/telegram.test.ts b/src/channels/plugins/outbound/telegram.test.ts new file mode 100644 index 000000000..13668f752 --- /dev/null +++ b/src/channels/plugins/outbound/telegram.test.ts @@ -0,0 +1,116 @@ +import { describe, expect, it, vi } from "vitest"; +import type { ReplyPayload } from "../../../auto-reply/types.js"; +import { telegramOutbound } from "./telegram.js"; + +describe("telegramOutbound", () => { + it("passes parsed reply/thread ids for sendText", async () => { + const sendTelegram = vi.fn().mockResolvedValue({ messageId: "tg-text-1", chatId: "123" }); + const sendText = telegramOutbound.sendText; + expect(sendText).toBeDefined(); + + const result = await sendText!({ + cfg: {}, + to: "123", + text: "hello", + accountId: "work", + replyToId: "44", + threadId: "55", + deps: { sendTelegram }, + }); + + expect(sendTelegram).toHaveBeenCalledWith( + "123", + "hello", + expect.objectContaining({ + textMode: "html", + verbose: false, + accountId: "work", + replyToMessageId: 44, + messageThreadId: 55, + }), + ); + expect(result).toEqual({ channel: "telegram", messageId: "tg-text-1", chatId: "123" }); + }); + + it("passes media options for sendMedia", async () => { + const sendTelegram = vi.fn().mockResolvedValue({ messageId: "tg-media-1", chatId: "123" }); + const sendMedia = telegramOutbound.sendMedia; + expect(sendMedia).toBeDefined(); + + const result = await sendMedia!({ + cfg: {}, + to: "123", + text: "caption", + mediaUrl: "https://example.com/a.jpg", + mediaLocalRoots: ["/tmp/media"], + accountId: "default", + deps: { sendTelegram }, + }); + + expect(sendTelegram).toHaveBeenCalledWith( + "123", + "caption", + expect.objectContaining({ + textMode: "html", + verbose: false, + mediaUrl: "https://example.com/a.jpg", + mediaLocalRoots: ["/tmp/media"], + }), + ); + expect(result).toEqual({ channel: "telegram", messageId: "tg-media-1", chatId: "123" }); + }); + + it("sends payload media list and applies buttons only to first message", async () => { + const sendTelegram = vi + .fn() + .mockResolvedValueOnce({ messageId: "tg-1", chatId: "123" }) + .mockResolvedValueOnce({ messageId: "tg-2", chatId: "123" }); + const sendPayload = telegramOutbound.sendPayload; + expect(sendPayload).toBeDefined(); + + const payload: ReplyPayload = { + text: "caption", + mediaUrls: ["https://example.com/1.jpg", "https://example.com/2.jpg"], + channelData: { + telegram: { + quoteText: "quoted", + buttons: [[{ text: "Approve", callback_data: "ok" }]], + }, + }, + }; + + const result = await sendPayload!({ + cfg: {}, + to: "123", + text: "", + payload, + mediaLocalRoots: ["/tmp/media"], + accountId: "default", + deps: { sendTelegram }, + }); + + expect(sendTelegram).toHaveBeenCalledTimes(2); + expect(sendTelegram).toHaveBeenNthCalledWith( + 1, + "123", + "caption", + expect.objectContaining({ + mediaUrl: "https://example.com/1.jpg", + quoteText: "quoted", + buttons: [[{ text: "Approve", callback_data: "ok" }]], + }), + ); + expect(sendTelegram).toHaveBeenNthCalledWith( + 2, + "123", + "", + expect.objectContaining({ + mediaUrl: "https://example.com/2.jpg", + quoteText: "quoted", + }), + ); + const secondCallOpts = sendTelegram.mock.calls[1]?.[2] as Record; + expect(secondCallOpts?.buttons).toBeUndefined(); + expect(result).toEqual({ channel: "telegram", messageId: "tg-2", chatId: "123" }); + }); +}); diff --git a/src/channels/plugins/outbound/telegram.ts b/src/channels/plugins/outbound/telegram.ts index 822452feb..32aadb8fb 100644 --- a/src/channels/plugins/outbound/telegram.ts +++ b/src/channels/plugins/outbound/telegram.ts @@ -1,3 +1,4 @@ +import type { OutboundSendDeps } from "../../../infra/outbound/deliver.js"; import type { TelegramInlineButtons } from "../../../telegram/button-types.js"; import { markdownToTelegramHtmlChunks } from "../../../telegram/format.js"; import { @@ -7,21 +8,48 @@ import { import { sendMessageTelegram } from "../../../telegram/send.js"; import type { ChannelOutboundAdapter } from "../types.js"; +function resolveTelegramSendContext(params: { + deps?: OutboundSendDeps; + accountId?: string | null; + replyToId?: string | null; + threadId?: string | number | null; +}): { + send: typeof sendMessageTelegram; + baseOpts: { + verbose: false; + textMode: "html"; + messageThreadId?: number; + replyToMessageId?: number; + accountId?: string; + }; +} { + const send = params.deps?.sendTelegram ?? sendMessageTelegram; + return { + send, + baseOpts: { + verbose: false, + textMode: "html", + messageThreadId: parseTelegramThreadId(params.threadId), + replyToMessageId: parseTelegramReplyToMessageId(params.replyToId), + accountId: params.accountId ?? undefined, + }, + }; +} + export const telegramOutbound: ChannelOutboundAdapter = { deliveryMode: "direct", chunker: markdownToTelegramHtmlChunks, chunkerMode: "markdown", textChunkLimit: 4000, sendText: async ({ to, text, accountId, deps, replyToId, threadId }) => { - const send = deps?.sendTelegram ?? sendMessageTelegram; - const replyToMessageId = parseTelegramReplyToMessageId(replyToId); - const messageThreadId = parseTelegramThreadId(threadId); + const { send, baseOpts } = resolveTelegramSendContext({ + deps, + accountId, + replyToId, + threadId, + }); const result = await send(to, text, { - verbose: false, - textMode: "html", - messageThreadId, - replyToMessageId, - accountId: accountId ?? undefined, + ...baseOpts, }); return { channel: "telegram", ...result }; }, @@ -35,24 +63,26 @@ export const telegramOutbound: ChannelOutboundAdapter = { replyToId, threadId, }) => { - const send = deps?.sendTelegram ?? sendMessageTelegram; - const replyToMessageId = parseTelegramReplyToMessageId(replyToId); - const messageThreadId = parseTelegramThreadId(threadId); + const { send, baseOpts } = resolveTelegramSendContext({ + deps, + accountId, + replyToId, + threadId, + }); const result = await send(to, text, { - verbose: false, + ...baseOpts, mediaUrl, - textMode: "html", - messageThreadId, - replyToMessageId, - accountId: accountId ?? undefined, mediaLocalRoots, }); return { channel: "telegram", ...result }; }, sendPayload: async ({ to, payload, mediaLocalRoots, accountId, deps, replyToId, threadId }) => { - const send = deps?.sendTelegram ?? sendMessageTelegram; - const replyToMessageId = parseTelegramReplyToMessageId(replyToId); - const messageThreadId = parseTelegramThreadId(threadId); + const { send, baseOpts: contextOpts } = resolveTelegramSendContext({ + deps, + accountId, + replyToId, + threadId, + }); const telegramData = payload.channelData?.telegram as | { buttons?: TelegramInlineButtons; quoteText?: string } | undefined; @@ -64,19 +94,15 @@ export const telegramOutbound: ChannelOutboundAdapter = { : payload.mediaUrl ? [payload.mediaUrl] : []; - const baseOpts = { - verbose: false, - textMode: "html" as const, - messageThreadId, - replyToMessageId, + const payloadOpts = { + ...contextOpts, quoteText, - accountId: accountId ?? undefined, mediaLocalRoots, }; if (mediaUrls.length === 0) { const result = await send(to, text, { - ...baseOpts, + ...payloadOpts, buttons: telegramData?.buttons, }); return { channel: "telegram", ...result }; @@ -88,7 +114,7 @@ export const telegramOutbound: ChannelOutboundAdapter = { const mediaUrl = mediaUrls[i]; const isFirst = i === 0; finalResult = await send(to, isFirst ? text : "", { - ...baseOpts, + ...payloadOpts, mediaUrl, ...(isFirst ? { buttons: telegramData?.buttons } : {}), }); diff --git a/src/channels/plugins/plugins-channel.test.ts b/src/channels/plugins/plugins-channel.test.ts index 7c4c6ebf1..e6f0e800a 100644 --- a/src/channels/plugins/plugins-channel.test.ts +++ b/src/channels/plugins/plugins-channel.test.ts @@ -6,6 +6,13 @@ import { normalizeSignalAccountInput } from "./onboarding/signal.js"; import { telegramOutbound } from "./outbound/telegram.js"; import { whatsappOutbound } from "./outbound/whatsapp.js"; +function expectWhatsAppTargetResolutionError(result: unknown) { + expect(result).toEqual({ + ok: false, + error: expect.any(Error), + }); +} + describe("imessage target normalization", () => { it("preserves service prefixes for handles", () => { expect(normalizeIMessageMessagingTarget("sms:+1 (555) 222-3333")).toBe("sms:+15552223333"); @@ -149,10 +156,7 @@ describe("whatsappOutbound.resolveTarget", () => { mode: "implicit", }); - expect(result).toEqual({ - ok: false, - error: expect.any(Error), - }); + expectWhatsAppTargetResolutionError(result); }); it("returns error when implicit target is not in allowFrom", () => { @@ -162,10 +166,7 @@ describe("whatsappOutbound.resolveTarget", () => { mode: "implicit", }); - expect(result).toEqual({ - ok: false, - error: expect.any(Error), - }); + expectWhatsAppTargetResolutionError(result); }); it("keeps group JID targets even when allowFrom does not contain them", () => { diff --git a/src/channels/plugins/plugins-core.test.ts b/src/channels/plugins/plugins-core.test.ts index f31d83f3e..d71080707 100644 --- a/src/channels/plugins/plugins-core.test.ts +++ b/src/channels/plugins/plugins-core.test.ts @@ -2,6 +2,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, expectTypeOf, it } from "vitest"; +import type { OpenClawConfig } from "../../config/config.js"; import type { DiscordProbe } from "../../discord/probe.js"; import type { DiscordTokenResolution } from "../../discord/token.js"; import type { IMessageProbe } from "../../imessage/probe.js"; @@ -11,7 +12,11 @@ import type { SignalProbe } from "../../signal/probe.js"; import type { SlackProbe } from "../../slack/probe.js"; import type { TelegramProbe } from "../../telegram/probe.js"; import type { TelegramTokenResolution } from "../../telegram/token.js"; -import { createTestRegistry } from "../../test-utils/channel-plugins.js"; +import { + createChannelTestPluginBase, + createOutboundTestPlugin, + createTestRegistry, +} from "../../test-utils/channel-plugins.js"; import { getChannelPluginCatalogEntry, listChannelPluginCatalogEntries } from "./catalog.js"; import { resolveChannelConfigWrites } from "./config-writes.js"; import { @@ -27,7 +32,7 @@ import { import { listChannelPlugins } from "./index.js"; import { loadChannelPlugin } from "./load.js"; import { loadChannelOutboundAdapter } from "./outbound/load.js"; -import type { ChannelOutboundAdapter, ChannelPlugin } from "./types.js"; +import type { ChannelDirectoryEntry, ChannelOutboundAdapter, ChannelPlugin } from "./types.js"; import type { BaseProbeResult, BaseTokenResolution } from "./types.js"; describe("channel plugin registry", () => { @@ -147,6 +152,71 @@ const registryWithMSTeams = createTestRegistry([ { pluginId: "msteams", plugin: msteamsPlugin, source: "test" }, ]); +const msteamsOutboundV2: ChannelOutboundAdapter = { + deliveryMode: "direct", + sendText: async () => ({ channel: "msteams", messageId: "m3" }), + sendMedia: async () => ({ channel: "msteams", messageId: "m4" }), +}; + +const msteamsPluginV2 = createOutboundTestPlugin({ + id: "msteams", + label: "Microsoft Teams", + outbound: msteamsOutboundV2, +}); + +const registryWithMSTeamsV2 = createTestRegistry([ + { pluginId: "msteams", plugin: msteamsPluginV2, source: "test-v2" }, +]); + +const mstNoOutboundPlugin = createChannelTestPluginBase({ + id: "msteams", + label: "Microsoft Teams", +}); + +const registryWithMSTeamsNoOutbound = createTestRegistry([ + { pluginId: "msteams", plugin: mstNoOutboundPlugin, source: "test-no-outbound" }, +]); + +function makeSlackConfigWritesCfg(accountIdKey: string) { + return { + channels: { + slack: { + configWrites: true, + accounts: { + [accountIdKey]: { configWrites: false }, + }, + }, + }, + }; +} + +type DirectoryListFn = (params: { + cfg: OpenClawConfig; + accountId?: string | null; + query?: string | null; + limit?: number | null; +}) => Promise; + +async function listDirectoryEntriesWithDefaults(listFn: DirectoryListFn, cfg: OpenClawConfig) { + return await listFn({ + cfg, + accountId: "default", + query: null, + limit: null, + }); +} + +async function expectDirectoryIds( + listFn: DirectoryListFn, + cfg: OpenClawConfig, + expected: string[], + options?: { sorted?: boolean }, +) { + const entries = await listDirectoryEntriesWithDefaults(listFn, cfg); + const ids = entries.map((entry) => entry.id); + expect(options?.sorted ? ids.toSorted() : ids).toEqual(expected); +} + describe("channel plugin loader", () => { beforeEach(() => { setActivePluginRegistry(emptyRegistry); @@ -167,6 +237,25 @@ describe("channel plugin loader", () => { const outbound = await loadChannelOutboundAdapter("msteams"); expect(outbound).toBe(msteamsOutbound); }); + + it("refreshes cached plugin values when registry changes", async () => { + setActivePluginRegistry(registryWithMSTeams); + expect(await loadChannelPlugin("msteams")).toBe(msteamsPlugin); + setActivePluginRegistry(registryWithMSTeamsV2); + expect(await loadChannelPlugin("msteams")).toBe(msteamsPluginV2); + }); + + it("refreshes cached outbound values when registry changes", async () => { + setActivePluginRegistry(registryWithMSTeams); + expect(await loadChannelOutboundAdapter("msteams")).toBe(msteamsOutbound); + setActivePluginRegistry(registryWithMSTeamsV2); + expect(await loadChannelOutboundAdapter("msteams")).toBe(msteamsOutboundV2); + }); + + it("returns undefined when plugin has no outbound adapter", async () => { + setActivePluginRegistry(registryWithMSTeamsNoOutbound); + expect(await loadChannelOutboundAdapter("msteams")).toBeUndefined(); + }); }); describe("BaseProbeResult assignability", () => { @@ -196,11 +285,8 @@ describe("BaseProbeResult assignability", () => { }); describe("BaseTokenResolution assignability", () => { - it("TelegramTokenResolution satisfies BaseTokenResolution", () => { + it("Telegram and Discord token resolutions satisfy BaseTokenResolution", () => { expectTypeOf().toMatchTypeOf(); - }); - - it("DiscordTokenResolution satisfies BaseTokenResolution", () => { expectTypeOf().toMatchTypeOf(); }); }); @@ -217,30 +303,12 @@ describe("resolveChannelConfigWrites", () => { }); it("account override wins over channel default", () => { - const cfg = { - channels: { - slack: { - configWrites: true, - accounts: { - work: { configWrites: false }, - }, - }, - }, - }; + const cfg = makeSlackConfigWritesCfg("work"); expect(resolveChannelConfigWrites({ cfg, channelId: "slack", accountId: "work" })).toBe(false); }); it("matches account ids case-insensitively", () => { - const cfg = { - channels: { - slack: { - configWrites: true, - accounts: { - Work: { configWrites: false }, - }, - }, - }, - }; + const cfg = makeSlackConfigWritesCfg("Work"); expect(resolveChannelConfigWrites({ cfg, channelId: "slack", accountId: "work" })).toBe(false); }); }); @@ -260,26 +328,13 @@ describe("directory (config-backed)", () => { // oxlint-disable-next-line typescript/no-explicit-any } as any; - const peers = await listSlackDirectoryPeersFromConfig({ + await expectDirectoryIds( + listSlackDirectoryPeersFromConfig, cfg, - accountId: "default", - query: null, - limit: null, - }); - expect(peers?.map((e) => e.id).toSorted()).toEqual([ - "user:u123", - "user:u234", - "user:u777", - "user:u999", - ]); - - const groups = await listSlackDirectoryGroupsFromConfig({ - cfg, - accountId: "default", - query: null, - limit: null, - }); - expect(groups?.map((e) => e.id)).toEqual(["channel:c111"]); + ["user:u123", "user:u234", "user:u777", "user:u999"], + { sorted: true }, + ); + await expectDirectoryIds(listSlackDirectoryGroupsFromConfig, cfg, ["channel:c111"]); }); it("lists Discord peers/groups from config (numeric ids only)", async () => { @@ -287,13 +342,14 @@ describe("directory (config-backed)", () => { channels: { discord: { token: "discord-test", - dm: { allowFrom: ["<@111>", "nope"] }, + dm: { allowFrom: ["<@111>", "<@!333>", "nope"] }, dms: { "222": {} }, guilds: { "123": { - users: ["<@12345>", "not-an-id"], + users: ["<@12345>", " discord:444 ", "not-an-id"], channels: { "555": {}, + "<#777>": {}, "channel:666": {}, general: {}, }, @@ -304,21 +360,18 @@ describe("directory (config-backed)", () => { // oxlint-disable-next-line typescript/no-explicit-any } as any; - const peers = await listDiscordDirectoryPeersFromConfig({ + await expectDirectoryIds( + listDiscordDirectoryPeersFromConfig, cfg, - accountId: "default", - query: null, - limit: null, - }); - expect(peers?.map((e) => e.id).toSorted()).toEqual(["user:111", "user:12345", "user:222"]); - - const groups = await listDiscordDirectoryGroupsFromConfig({ + ["user:111", "user:12345", "user:222", "user:333", "user:444"], + { sorted: true }, + ); + await expectDirectoryIds( + listDiscordDirectoryGroupsFromConfig, cfg, - accountId: "default", - query: null, - limit: null, - }); - expect(groups?.map((e) => e.id).toSorted()).toEqual(["channel:555", "channel:666"]); + ["channel:555", "channel:666", "channel:777"], + { sorted: true }, + ); }); it("lists Telegram peers/groups from config", async () => { @@ -334,21 +387,15 @@ describe("directory (config-backed)", () => { // oxlint-disable-next-line typescript/no-explicit-any } as any; - const peers = await listTelegramDirectoryPeersFromConfig({ + await expectDirectoryIds( + listTelegramDirectoryPeersFromConfig, cfg, - accountId: "default", - query: null, - limit: null, - }); - expect(peers?.map((e) => e.id).toSorted()).toEqual(["123", "456", "@alice", "@bob"]); - - const groups = await listTelegramDirectoryGroupsFromConfig({ - cfg, - accountId: "default", - query: null, - limit: null, - }); - expect(groups?.map((e) => e.id)).toEqual(["-1001"]); + ["123", "456", "@alice", "@bob"], + { + sorted: true, + }, + ); + await expectDirectoryIds(listTelegramDirectoryGroupsFromConfig, cfg, ["-1001"]); }); it("lists WhatsApp peers/groups from config", async () => { @@ -362,21 +409,8 @@ describe("directory (config-backed)", () => { // oxlint-disable-next-line typescript/no-explicit-any } as any; - const peers = await listWhatsAppDirectoryPeersFromConfig({ - cfg, - accountId: "default", - query: null, - limit: null, - }); - expect(peers?.map((e) => e.id)).toEqual(["+15550000000"]); - - const groups = await listWhatsAppDirectoryGroupsFromConfig({ - cfg, - accountId: "default", - query: null, - limit: null, - }); - expect(groups?.map((e) => e.id)).toEqual(["999@g.us"]); + await expectDirectoryIds(listWhatsAppDirectoryPeersFromConfig, cfg, ["+15550000000"]); + await expectDirectoryIds(listWhatsAppDirectoryGroupsFromConfig, cfg, ["999@g.us"]); }); it("applies query and limit filtering for config-backed directories", async () => { diff --git a/src/channels/plugins/registry-loader.ts b/src/channels/plugins/registry-loader.ts new file mode 100644 index 000000000..9f23c5fa0 --- /dev/null +++ b/src/channels/plugins/registry-loader.ts @@ -0,0 +1,35 @@ +import type { PluginChannelRegistration, PluginRegistry } from "../../plugins/registry.js"; +import { getActivePluginRegistry } from "../../plugins/runtime.js"; +import type { ChannelId } from "./types.js"; + +type ChannelRegistryValueResolver = ( + entry: PluginChannelRegistration, +) => TValue | undefined; + +export function createChannelRegistryLoader( + resolveValue: ChannelRegistryValueResolver, +): (id: ChannelId) => Promise { + const cache = new Map(); + let lastRegistry: PluginRegistry | null = null; + + return async (id: ChannelId): Promise => { + const registry = getActivePluginRegistry(); + if (registry !== lastRegistry) { + cache.clear(); + lastRegistry = registry; + } + const cached = cache.get(id); + if (cached) { + return cached; + } + const pluginEntry = registry?.channels.find((entry) => entry.plugin.id === id); + if (!pluginEntry) { + return undefined; + } + const resolved = resolveValue(pluginEntry); + if (resolved) { + cache.set(id, resolved); + } + return resolved; + }; +} diff --git a/src/channels/plugins/status-issues/bluebubbles.test.ts b/src/channels/plugins/status-issues/bluebubbles.test.ts new file mode 100644 index 000000000..4613daa15 --- /dev/null +++ b/src/channels/plugins/status-issues/bluebubbles.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from "vitest"; +import { collectBlueBubblesStatusIssues } from "./bluebubbles.js"; + +describe("collectBlueBubblesStatusIssues", () => { + it("reports unconfigured enabled accounts", () => { + const issues = collectBlueBubblesStatusIssues([ + { + accountId: "default", + enabled: true, + configured: false, + }, + ]); + + expect(issues).toEqual([ + expect.objectContaining({ + channel: "bluebubbles", + accountId: "default", + kind: "config", + }), + ]); + }); + + it("reports probe failure and runtime error for configured running accounts", () => { + const issues = collectBlueBubblesStatusIssues([ + { + accountId: "work", + enabled: true, + configured: true, + running: true, + lastError: "timeout", + probe: { + ok: false, + status: 503, + }, + }, + ]); + + expect(issues).toHaveLength(2); + expect(issues[0]).toEqual( + expect.objectContaining({ + channel: "bluebubbles", + accountId: "work", + kind: "runtime", + }), + ); + expect(issues[1]).toEqual( + expect.objectContaining({ + channel: "bluebubbles", + accountId: "work", + kind: "runtime", + message: "Channel error: timeout", + }), + ); + }); + + it("skips disabled accounts", () => { + const issues = collectBlueBubblesStatusIssues([ + { + accountId: "disabled", + enabled: false, + configured: false, + }, + ]); + expect(issues).toEqual([]); + }); +}); diff --git a/src/channels/plugins/status-issues/bluebubbles.ts b/src/channels/plugins/status-issues/bluebubbles.ts index 967226438..c37f45bc7 100644 --- a/src/channels/plugins/status-issues/bluebubbles.ts +++ b/src/channels/plugins/status-issues/bluebubbles.ts @@ -1,5 +1,5 @@ import type { ChannelAccountSnapshot, ChannelStatusIssue } from "../types.js"; -import { asString, isRecord } from "./shared.js"; +import { asString, collectIssuesForEnabledAccounts, isRecord } from "./shared.js"; type BlueBubblesAccountStatus = { accountId?: unknown; @@ -48,61 +48,53 @@ function readBlueBubblesProbeResult(value: unknown): BlueBubblesProbeResult | nu export function collectBlueBubblesStatusIssues( accounts: ChannelAccountSnapshot[], ): ChannelStatusIssue[] { - const issues: ChannelStatusIssue[] = []; - for (const entry of accounts) { - const account = readBlueBubblesAccountStatus(entry); - if (!account) { - continue; - } - const accountId = asString(account.accountId) ?? "default"; - const enabled = account.enabled !== false; - if (!enabled) { - continue; - } + return collectIssuesForEnabledAccounts({ + accounts, + readAccount: readBlueBubblesAccountStatus, + collectIssues: ({ account, accountId, issues }) => { + const configured = account.configured === true; + const running = account.running === true; + const lastError = asString(account.lastError); + const probe = readBlueBubblesProbeResult(account.probe); - const configured = account.configured === true; - const running = account.running === true; - const lastError = asString(account.lastError); - const probe = readBlueBubblesProbeResult(account.probe); + // Check for unconfigured accounts + if (!configured) { + issues.push({ + channel: "bluebubbles", + accountId, + kind: "config", + message: "Not configured (missing serverUrl or password).", + fix: "Run: openclaw channels add bluebubbles --http-url --password ", + }); + return; + } - // Check for unconfigured accounts - if (!configured) { - issues.push({ - channel: "bluebubbles", - accountId, - kind: "config", - message: "Not configured (missing serverUrl or password).", - fix: "Run: openclaw channels add bluebubbles --http-url --password ", - }); - continue; - } + // Check for probe failures + if (probe && probe.ok === false) { + const errorDetail = probe.error + ? `: ${probe.error}` + : probe.status + ? ` (HTTP ${probe.status})` + : ""; + issues.push({ + channel: "bluebubbles", + accountId, + kind: "runtime", + message: `BlueBubbles server unreachable${errorDetail}`, + fix: "Check that the BlueBubbles server is running and accessible. Verify serverUrl and password in your config.", + }); + } - // Check for probe failures - if (probe && probe.ok === false) { - const errorDetail = probe.error - ? `: ${probe.error}` - : probe.status - ? ` (HTTP ${probe.status})` - : ""; - issues.push({ - channel: "bluebubbles", - accountId, - kind: "runtime", - message: `BlueBubbles server unreachable${errorDetail}`, - fix: "Check that the BlueBubbles server is running and accessible. Verify serverUrl and password in your config.", - }); - } - - // Check for runtime errors - if (running && lastError) { - issues.push({ - channel: "bluebubbles", - accountId, - kind: "runtime", - message: `Channel error: ${lastError}`, - fix: "Check gateway logs for details. If the webhook is failing, verify the webhook URL is configured in BlueBubbles server settings.", - }); - } - } - return issues; + // Check for runtime errors + if (running && lastError) { + issues.push({ + channel: "bluebubbles", + accountId, + kind: "runtime", + message: `Channel error: ${lastError}`, + fix: "Check gateway logs for details. If the webhook is failing, verify the webhook URL is configured in BlueBubbles server settings.", + }); + } + }, + }); } diff --git a/src/channels/plugins/status-issues/shared.ts b/src/channels/plugins/status-issues/shared.ts index d4f5be878..8a6377afc 100644 --- a/src/channels/plugins/status-issues/shared.ts +++ b/src/channels/plugins/status-issues/shared.ts @@ -1,4 +1,5 @@ import { isRecord } from "../../../utils.js"; +import type { ChannelAccountSnapshot, ChannelStatusIssue } from "../types.js"; export { isRecord }; export function asString(value: unknown): string | undefined { @@ -41,3 +42,22 @@ export function resolveEnabledConfiguredAccountId(account: { const configured = account.configured === true; return enabled && configured ? accountId : null; } + +export function collectIssuesForEnabledAccounts< + T extends { accountId?: unknown; enabled?: unknown }, +>(params: { + accounts: ChannelAccountSnapshot[]; + readAccount: (value: ChannelAccountSnapshot) => T | null; + collectIssues: (params: { account: T; accountId: string; issues: ChannelStatusIssue[] }) => void; +}): ChannelStatusIssue[] { + const issues: ChannelStatusIssue[] = []; + for (const entry of params.accounts) { + const account = params.readAccount(entry); + if (!account || account.enabled === false) { + continue; + } + const accountId = asString(account.accountId) ?? "default"; + params.collectIssues({ account, accountId, issues }); + } + return issues; +} diff --git a/src/channels/plugins/status-issues/whatsapp.test.ts b/src/channels/plugins/status-issues/whatsapp.test.ts new file mode 100644 index 000000000..77a4e6ecf --- /dev/null +++ b/src/channels/plugins/status-issues/whatsapp.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from "vitest"; +import { collectWhatsAppStatusIssues } from "./whatsapp.js"; + +describe("collectWhatsAppStatusIssues", () => { + it("reports unlinked enabled accounts", () => { + const issues = collectWhatsAppStatusIssues([ + { + accountId: "default", + enabled: true, + linked: false, + }, + ]); + + expect(issues).toEqual([ + expect.objectContaining({ + channel: "whatsapp", + accountId: "default", + kind: "auth", + }), + ]); + }); + + it("reports linked but disconnected runtime state", () => { + const issues = collectWhatsAppStatusIssues([ + { + accountId: "work", + enabled: true, + linked: true, + running: true, + connected: false, + reconnectAttempts: 2, + lastError: "socket closed", + }, + ]); + + expect(issues).toEqual([ + expect.objectContaining({ + channel: "whatsapp", + accountId: "work", + kind: "runtime", + message: "Linked but disconnected (reconnectAttempts=2): socket closed", + }), + ]); + }); + + it("skips disabled accounts", () => { + const issues = collectWhatsAppStatusIssues([ + { + accountId: "disabled", + enabled: false, + linked: false, + }, + ]); + expect(issues).toEqual([]); + }); +}); diff --git a/src/channels/plugins/status-issues/whatsapp.ts b/src/channels/plugins/status-issues/whatsapp.ts index 99ed65a00..4e1c7c7b0 100644 --- a/src/channels/plugins/status-issues/whatsapp.ts +++ b/src/channels/plugins/status-issues/whatsapp.ts @@ -1,6 +1,6 @@ import { formatCliCommand } from "../../../cli/command-format.js"; import type { ChannelAccountSnapshot, ChannelStatusIssue } from "../types.js"; -import { asString, isRecord } from "./shared.js"; +import { asString, collectIssuesForEnabledAccounts, isRecord } from "./shared.js"; type WhatsAppAccountStatus = { accountId?: unknown; @@ -30,44 +30,37 @@ function readWhatsAppAccountStatus(value: ChannelAccountSnapshot): WhatsAppAccou export function collectWhatsAppStatusIssues( accounts: ChannelAccountSnapshot[], ): ChannelStatusIssue[] { - const issues: ChannelStatusIssue[] = []; - for (const entry of accounts) { - const account = readWhatsAppAccountStatus(entry); - if (!account) { - continue; - } - const accountId = asString(account.accountId) ?? "default"; - const enabled = account.enabled !== false; - if (!enabled) { - continue; - } - const linked = account.linked === true; - const running = account.running === true; - const connected = account.connected === true; - const reconnectAttempts = - typeof account.reconnectAttempts === "number" ? account.reconnectAttempts : null; - const lastError = asString(account.lastError); + return collectIssuesForEnabledAccounts({ + accounts, + readAccount: readWhatsAppAccountStatus, + collectIssues: ({ account, accountId, issues }) => { + const linked = account.linked === true; + const running = account.running === true; + const connected = account.connected === true; + const reconnectAttempts = + typeof account.reconnectAttempts === "number" ? account.reconnectAttempts : null; + const lastError = asString(account.lastError); - if (!linked) { - issues.push({ - channel: "whatsapp", - accountId, - kind: "auth", - message: "Not linked (no WhatsApp Web session).", - fix: `Run: ${formatCliCommand("openclaw channels login")} (scan QR on the gateway host).`, - }); - continue; - } + if (!linked) { + issues.push({ + channel: "whatsapp", + accountId, + kind: "auth", + message: "Not linked (no WhatsApp Web session).", + fix: `Run: ${formatCliCommand("openclaw channels login")} (scan QR on the gateway host).`, + }); + return; + } - if (running && !connected) { - issues.push({ - channel: "whatsapp", - accountId, - kind: "runtime", - message: `Linked but disconnected${reconnectAttempts != null ? ` (reconnectAttempts=${reconnectAttempts})` : ""}${lastError ? `: ${lastError}` : "."}`, - fix: `Run: ${formatCliCommand("openclaw doctor")} (or restart the gateway). If it persists, relink via channels login and check logs.`, - }); - } - } - return issues; + if (running && !connected) { + issues.push({ + channel: "whatsapp", + accountId, + kind: "runtime", + message: `Linked but disconnected${reconnectAttempts != null ? ` (reconnectAttempts=${reconnectAttempts})` : ""}${lastError ? `: ${lastError}` : "."}`, + fix: `Run: ${formatCliCommand("openclaw doctor")} (or restart the gateway). If it persists, relink via channels login and check logs.`, + }); + } + }, + }); } diff --git a/src/channels/plugins/types.adapters.ts b/src/channels/plugins/types.adapters.ts index ce0f9bbb8..113df6ad5 100644 --- a/src/channels/plugins/types.adapters.ts +++ b/src/channels/plugins/types.adapters.ts @@ -237,47 +237,37 @@ export type ChannelHeartbeatAdapter = { }; }; +type ChannelDirectorySelfParams = { + cfg: OpenClawConfig; + accountId?: string | null; + runtime: RuntimeEnv; +}; + +type ChannelDirectoryListParams = { + cfg: OpenClawConfig; + accountId?: string | null; + query?: string | null; + limit?: number | null; + runtime: RuntimeEnv; +}; + +type ChannelDirectoryListGroupMembersParams = { + cfg: OpenClawConfig; + accountId?: string | null; + groupId: string; + limit?: number | null; + runtime: RuntimeEnv; +}; + export type ChannelDirectoryAdapter = { - self?: (params: { - cfg: OpenClawConfig; - accountId?: string | null; - runtime: RuntimeEnv; - }) => Promise; - listPeers?: (params: { - cfg: OpenClawConfig; - accountId?: string | null; - query?: string | null; - limit?: number | null; - runtime: RuntimeEnv; - }) => Promise; - listPeersLive?: (params: { - cfg: OpenClawConfig; - accountId?: string | null; - query?: string | null; - limit?: number | null; - runtime: RuntimeEnv; - }) => Promise; - listGroups?: (params: { - cfg: OpenClawConfig; - accountId?: string | null; - query?: string | null; - limit?: number | null; - runtime: RuntimeEnv; - }) => Promise; - listGroupsLive?: (params: { - cfg: OpenClawConfig; - accountId?: string | null; - query?: string | null; - limit?: number | null; - runtime: RuntimeEnv; - }) => Promise; - listGroupMembers?: (params: { - cfg: OpenClawConfig; - accountId?: string | null; - groupId: string; - limit?: number | null; - runtime: RuntimeEnv; - }) => Promise; + self?: (params: ChannelDirectorySelfParams) => Promise; + listPeers?: (params: ChannelDirectoryListParams) => Promise; + listPeersLive?: (params: ChannelDirectoryListParams) => Promise; + listGroups?: (params: ChannelDirectoryListParams) => Promise; + listGroupsLive?: (params: ChannelDirectoryListParams) => Promise; + listGroupMembers?: ( + params: ChannelDirectoryListGroupMembersParams, + ) => Promise; }; export type ChannelResolveKind = "user" | "group"; diff --git a/src/channels/plugins/whatsapp-heartbeat.test.ts b/src/channels/plugins/whatsapp-heartbeat.test.ts index 5ec7215c0..f4b0945a4 100644 --- a/src/channels/plugins/whatsapp-heartbeat.test.ts +++ b/src/channels/plugins/whatsapp-heartbeat.test.ts @@ -38,6 +38,13 @@ describe("resolveWhatsAppHeartbeatRecipients", () => { return resolveWhatsAppHeartbeatRecipients(makeCfg(cfgOverrides), opts); } + function setSingleUnauthorizedSessionWithAllowFrom() { + setSessionStore({ + a: { lastChannel: "whatsapp", lastTo: "+15550000099", updatedAt: 2, sessionId: "a" }, + }); + setAllowFromStore(["+15550000001"]); + } + beforeEach(() => { vi.mocked(loadSessionStore).mockClear(); vi.mocked(readChannelAllowFromStoreSync).mockClear(); @@ -57,10 +64,7 @@ describe("resolveWhatsAppHeartbeatRecipients", () => { }); it("falls back to allowFrom when no session recipient is authorized", () => { - setSessionStore({ - a: { lastChannel: "whatsapp", lastTo: "+15550000099", updatedAt: 2, sessionId: "a" }, - }); - setAllowFromStore(["+15550000001"]); + setSingleUnauthorizedSessionWithAllowFrom(); const result = resolveWith(); @@ -68,10 +72,7 @@ describe("resolveWhatsAppHeartbeatRecipients", () => { }); it("includes both session and allowFrom recipients when --all is set", () => { - setSessionStore({ - a: { lastChannel: "whatsapp", lastTo: "+15550000099", updatedAt: 2, sessionId: "a" }, - }); - setAllowFromStore(["+15550000001"]); + setSingleUnauthorizedSessionWithAllowFrom(); const result = resolveWith({}, { all: true }); diff --git a/src/channels/sender-label.test.ts b/src/channels/sender-label.test.ts new file mode 100644 index 000000000..3290be52c --- /dev/null +++ b/src/channels/sender-label.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from "vitest"; +import { listSenderLabelCandidates, resolveSenderLabel } from "./sender-label.js"; + +describe("resolveSenderLabel", () => { + it("prefers display + identifier when both are available", () => { + expect( + resolveSenderLabel({ + name: " Alice ", + e164: " +15551234567 ", + }), + ).toBe("Alice (+15551234567)"); + }); + + it("falls back to identifier-only labels", () => { + expect( + resolveSenderLabel({ + id: " user-123 ", + }), + ).toBe("user-123"); + }); + + it("returns null when all values are empty", () => { + expect( + resolveSenderLabel({ + name: " ", + username: "", + tag: " ", + }), + ).toBeNull(); + }); +}); + +describe("listSenderLabelCandidates", () => { + it("returns unique normalized candidates plus resolved label", () => { + expect( + listSenderLabelCandidates({ + name: "Alice", + username: "alice", + tag: "alice", + e164: "+15551234567", + id: "user-123", + }), + ).toEqual(["Alice", "alice", "+15551234567", "user-123", "Alice (+15551234567)"]); + }); +}); diff --git a/src/channels/sender-label.ts b/src/channels/sender-label.ts index 208c5d5a4..e8d4132f0 100644 --- a/src/channels/sender-label.ts +++ b/src/channels/sender-label.ts @@ -11,12 +11,18 @@ function normalize(value?: string): string | undefined { return trimmed ? trimmed : undefined; } +function normalizeSenderLabelParams(params: SenderLabelParams) { + return { + name: normalize(params.name), + username: normalize(params.username), + tag: normalize(params.tag), + e164: normalize(params.e164), + id: normalize(params.id), + }; +} + export function resolveSenderLabel(params: SenderLabelParams): string | null { - const name = normalize(params.name); - const username = normalize(params.username); - const tag = normalize(params.tag); - const e164 = normalize(params.e164); - const id = normalize(params.id); + const { name, username, tag, e164, id } = normalizeSenderLabelParams(params); const display = name ?? username ?? tag ?? ""; const idPart = e164 ?? id ?? ""; @@ -28,11 +34,7 @@ export function resolveSenderLabel(params: SenderLabelParams): string | null { export function listSenderLabelCandidates(params: SenderLabelParams): string[] { const candidates = new Set(); - const name = normalize(params.name); - const username = normalize(params.username); - const tag = normalize(params.tag); - const e164 = normalize(params.e164); - const id = normalize(params.id); + const { name, username, tag, e164, id } = normalizeSenderLabelParams(params); if (name) { candidates.add(name); diff --git a/src/channels/status-reactions.test.ts b/src/channels/status-reactions.test.ts index 144faed13..9b61946d6 100644 --- a/src/channels/status-reactions.test.ts +++ b/src/channels/status-reactions.test.ts @@ -357,56 +357,55 @@ describe("createStatusReactionController", () => { }, ] as const; + const createControllerAfterThinking = async () => { + const state = createEnabledController(); + void state.controller.setThinking(); + await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); + return state; + }; + for (const testCase of stallCases) { it(`should trigger ${testCase.name}`, async () => { - const { calls, controller } = createEnabledController(); - - void controller.setThinking(); - await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); + const { calls } = await createControllerAfterThinking(); await vi.advanceTimersByTimeAsync(testCase.delayMs); expect(calls).toContainEqual({ method: "set", emoji: testCase.expected }); }); } - it("should reset stall timers on phase change", async () => { - const { calls, controller } = createEnabledController(); + const stallResetCases = [ + { + name: "phase change", + runUpdate: (controller: ReturnType) => { + void controller.setTool("exec"); + return vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); + }, + }, + { + name: "repeated same-phase updates", + runUpdate: (controller: ReturnType) => { + void controller.setThinking(); + return Promise.resolve(); + }, + }, + ] as const; - void controller.setThinking(); - await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); + for (const testCase of stallResetCases) { + it(`should reset stall timers on ${testCase.name}`, async () => { + const { calls, controller } = await createControllerAfterThinking(); - // Advance halfway to soft stall - await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.stallSoftMs / 2); + // Advance halfway to soft stall. + await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.stallSoftMs / 2); - // Change phase - void controller.setTool("exec"); - await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); + await testCase.runUpdate(controller); - // Advance another halfway - should not trigger stall yet - await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.stallSoftMs / 2); + // Advance another halfway - should not trigger stall yet. + await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.stallSoftMs / 2); - const stallCalls = calls.filter((c) => c.emoji === DEFAULT_EMOJIS.stallSoft); - expect(stallCalls).toHaveLength(0); - }); - - it("should reset stall timers on repeated same-phase updates", async () => { - const { calls, controller } = createEnabledController(); - - void controller.setThinking(); - await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); - - // Advance halfway to soft stall - await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.stallSoftMs / 2); - - // Re-affirm same phase (should reset timers) - void controller.setThinking(); - - // Advance another halfway - should not trigger stall yet - await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.stallSoftMs / 2); - - const stallCalls = calls.filter((c) => c.emoji === DEFAULT_EMOJIS.stallSoft); - expect(stallCalls).toHaveLength(0); - }); + const stallCalls = calls.filter((c) => c.emoji === DEFAULT_EMOJIS.stallSoft); + expect(stallCalls).toHaveLength(0); + }); + } it("should call onError callback when adapter throws", async () => { const onError = vi.fn(); diff --git a/src/channels/status-reactions.ts b/src/channels/status-reactions.ts index 266f4199e..4b0651232 100644 --- a/src/channels/status-reactions.ts +++ b/src/channels/status-reactions.ts @@ -306,7 +306,7 @@ export function createStatusReactionController(params: { scheduleEmoji(emoji); } - function setDone(): Promise { + function finishWithEmoji(emoji: string): Promise { if (!enabled) { return Promise.resolve(); } @@ -316,24 +316,17 @@ export function createStatusReactionController(params: { // Directly enqueue to ensure we return the updated promise return enqueue(async () => { - await applyEmoji(emojis.done); + await applyEmoji(emoji); pendingEmoji = ""; }); } + function setDone(): Promise { + return finishWithEmoji(emojis.done); + } + function setError(): Promise { - if (!enabled) { - return Promise.resolve(); - } - - finished = true; - clearAllTimers(); - - // Directly enqueue to ensure we return the updated promise - return enqueue(async () => { - await applyEmoji(emojis.error); - pendingEmoji = ""; - }); + return finishWithEmoji(emojis.error); } async function clear(): Promise { diff --git a/src/slack/channel-migration.test.ts b/src/slack/channel-migration.test.ts index 86cc11542..047cc3c6d 100644 --- a/src/slack/channel-migration.test.ts +++ b/src/slack/channel-migration.test.ts @@ -1,17 +1,38 @@ import { describe, expect, it } from "vitest"; -import { migrateSlackChannelConfig } from "./channel-migration.js"; +import { migrateSlackChannelConfig, migrateSlackChannelsInPlace } from "./channel-migration.js"; -describe("migrateSlackChannelConfig", () => { - it("migrates global channel ids", () => { - const cfg = { - channels: { - slack: { - channels: { - C123: { requireMention: false }, +function createSlackGlobalChannelConfig(channels: Record>) { + return { + channels: { + slack: { + channels, + }, + }, + }; +} + +function createSlackAccountChannelConfig( + accountId: string, + channels: Record>, +) { + return { + channels: { + slack: { + accounts: { + [accountId]: { + channels, }, }, }, - }; + }, + }; +} + +describe("migrateSlackChannelConfig", () => { + it("migrates global channel ids", () => { + const cfg = createSlackGlobalChannelConfig({ + C123: { requireMention: false }, + }); const result = migrateSlackChannelConfig({ cfg, @@ -27,19 +48,9 @@ describe("migrateSlackChannelConfig", () => { }); it("migrates account-scoped channels", () => { - const cfg = { - channels: { - slack: { - accounts: { - primary: { - channels: { - C123: { requireMention: true }, - }, - }, - }, - }, - }, - }; + const cfg = createSlackAccountChannelConfig("primary", { + C123: { requireMention: true }, + }); const result = migrateSlackChannelConfig({ cfg, @@ -56,19 +67,9 @@ describe("migrateSlackChannelConfig", () => { }); it("matches account ids case-insensitively", () => { - const cfg = { - channels: { - slack: { - accounts: { - Primary: { - channels: { - C123: {}, - }, - }, - }, - }, - }, - }; + const cfg = createSlackAccountChannelConfig("Primary", { + C123: {}, + }); const result = migrateSlackChannelConfig({ cfg, @@ -84,16 +85,10 @@ describe("migrateSlackChannelConfig", () => { }); it("skips migration when new id already exists", () => { - const cfg = { - channels: { - slack: { - channels: { - C123: { requireMention: true }, - C999: { requireMention: false }, - }, - }, - }, - }; + const cfg = createSlackGlobalChannelConfig({ + C123: { requireMention: true }, + C999: { requireMention: false }, + }); const result = migrateSlackChannelConfig({ cfg, @@ -109,4 +104,15 @@ describe("migrateSlackChannelConfig", () => { C999: { requireMention: false }, }); }); + + it("no-ops when old and new channel ids are the same", () => { + const channels = { + C123: { requireMention: true }, + }; + const result = migrateSlackChannelsInPlace(channels, "C123", "C123"); + expect(result).toEqual({ migrated: false, skippedExisting: false }); + expect(channels).toEqual({ + C123: { requireMention: true }, + }); + }); }); diff --git a/src/telegram/group-migration.test.ts b/src/telegram/group-migration.test.ts index 4d4ca9758..12375f7cf 100644 --- a/src/telegram/group-migration.test.ts +++ b/src/telegram/group-migration.test.ts @@ -1,17 +1,38 @@ import { describe, expect, it } from "vitest"; -import { migrateTelegramGroupConfig } from "./group-migration.js"; +import { migrateTelegramGroupConfig, migrateTelegramGroupsInPlace } from "./group-migration.js"; -describe("migrateTelegramGroupConfig", () => { - it("migrates global group ids", () => { - const cfg = { - channels: { - telegram: { - groups: { - "-123": { requireMention: false }, +function createTelegramGlobalGroupConfig(groups: Record>) { + return { + channels: { + telegram: { + groups, + }, + }, + }; +} + +function createTelegramAccountGroupConfig( + accountId: string, + groups: Record>, +) { + return { + channels: { + telegram: { + accounts: { + [accountId]: { + groups, }, }, }, - }; + }, + }; +} + +describe("migrateTelegramGroupConfig", () => { + it("migrates global group ids", () => { + const cfg = createTelegramGlobalGroupConfig({ + "-123": { requireMention: false }, + }); const result = migrateTelegramGroupConfig({ cfg, @@ -27,19 +48,9 @@ describe("migrateTelegramGroupConfig", () => { }); it("migrates account-scoped groups", () => { - const cfg = { - channels: { - telegram: { - accounts: { - primary: { - groups: { - "-123": { requireMention: true }, - }, - }, - }, - }, - }, - }; + const cfg = createTelegramAccountGroupConfig("primary", { + "-123": { requireMention: true }, + }); const result = migrateTelegramGroupConfig({ cfg, @@ -56,19 +67,9 @@ describe("migrateTelegramGroupConfig", () => { }); it("matches account ids case-insensitively", () => { - const cfg = { - channels: { - telegram: { - accounts: { - Primary: { - groups: { - "-123": {}, - }, - }, - }, - }, - }, - }; + const cfg = createTelegramAccountGroupConfig("Primary", { + "-123": {}, + }); const result = migrateTelegramGroupConfig({ cfg, @@ -84,16 +85,10 @@ describe("migrateTelegramGroupConfig", () => { }); it("skips migration when new id already exists", () => { - const cfg = { - channels: { - telegram: { - groups: { - "-123": { requireMention: true }, - "-100123": { requireMention: false }, - }, - }, - }, - }; + const cfg = createTelegramGlobalGroupConfig({ + "-123": { requireMention: true }, + "-100123": { requireMention: false }, + }); const result = migrateTelegramGroupConfig({ cfg, @@ -109,4 +104,15 @@ describe("migrateTelegramGroupConfig", () => { "-100123": { requireMention: false }, }); }); + + it("no-ops when old and new group ids are the same", () => { + const groups = { + "-123": { requireMention: true }, + }; + const result = migrateTelegramGroupsInPlace(groups, "-123", "-123"); + expect(result).toEqual({ migrated: false, skippedExisting: false }); + expect(groups).toEqual({ + "-123": { requireMention: true }, + }); + }); });