From 72a7d6a8dc04e7c709e902638c1d43efe09adaef Mon Sep 17 00:00:00 2001 From: Liz Zhang Date: Mon, 25 May 2026 21:15:24 -0700 Subject: [PATCH] fix(whatsapp): warn once when group inbound dropped for missing channels.whatsapp.groups entry (#83833) Merged via squash. Prepared head SHA: 8fc52432101932c344bd95b184686ccce4fbca76 Co-authored-by: zhang-liz <13132583+zhang-liz@users.noreply.github.com> Co-authored-by: mcaxtr <7562095+mcaxtr@users.noreply.github.com> Reviewed-by: @mcaxtr --- .../group-gating.allowlist-warn.test.ts | 208 ++++++++++++++++++ .../group-gating.audio-preflight.test.ts | 2 +- .../src/auto-reply/monitor/group-gating.ts | 37 +++- .../auto-reply/web-auto-reply-monitor.test.ts | 2 +- extensions/whatsapp/src/group-config-path.ts | 98 +++++++++ extensions/whatsapp/src/shared.ts | 52 +---- 6 files changed, 345 insertions(+), 54 deletions(-) create mode 100644 extensions/whatsapp/src/auto-reply/monitor/group-gating.allowlist-warn.test.ts create mode 100644 extensions/whatsapp/src/group-config-path.ts diff --git a/extensions/whatsapp/src/auto-reply/monitor/group-gating.allowlist-warn.test.ts b/extensions/whatsapp/src/auto-reply/monitor/group-gating.allowlist-warn.test.ts new file mode 100644 index 000000000000..fdec0d865ba6 --- /dev/null +++ b/extensions/whatsapp/src/auto-reply/monitor/group-gating.allowlist-warn.test.ts @@ -0,0 +1,208 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("./group-activation.js", () => ({ + resolveGroupActivationFor: vi.fn(async () => "mention"), +})); + +import type { MentionConfig } from "../mentions.js"; +import type { WebInboundMsg } from "../types.js"; +import { + resetGroupDropWarningsForTests, + applyGroupGating, + type GroupHistoryEntry, +} from "./group-gating.js"; + +function makeUnregisteredGroupMsg( + conversationId: string, + accountId: string = "default", +): WebInboundMsg { + return { + id: `msg-${conversationId}`, + from: conversationId, + to: "+15550000001", + body: "@openclaw hello", + chatId: conversationId, + chatType: "group", + conversationId, + timestamp: 1700000000, + accountId, + sender: { e164: "+15550000002", name: "Alice" }, + } as WebInboundMsg; +} + +type WarnLogger = (obj: unknown, msg: string) => void; +type ApplyGroupGatingParams = Parameters[0]; + +function makeParams( + msg: WebInboundMsg, + warn: WarnLogger, + cfg: ApplyGroupGatingParams["cfg"] = { + channels: { + whatsapp: { + groupPolicy: "allowlist", + groups: { + "registered@g.us": {}, + }, + accounts: { + work: { + groupPolicy: "allowlist", + groups: { + "registered@g.us": {}, + }, + }, + }, + }, + }, + messages: { + groupChat: { + mentionPatterns: ["\\bopenclaw\\b"], + }, + }, + } as never, +) { + return { + cfg, + msg, + conversationId: msg.conversationId, + groupHistoryKey: `whatsapp:group:${msg.conversationId}`, + agentId: "main", + sessionKey: `agent:main:whatsapp:group:${msg.conversationId}`, + baseMentionConfig: { mentionRegexes: [/\bopenclaw\b/i] } satisfies MentionConfig, + groupHistories: new Map(), + groupHistoryLimit: 20, + groupMemberNames: new Map>(), + logVerbose: vi.fn(), + replyLogger: { debug: vi.fn(), warn }, + }; +} + +describe("applyGroupGating allowlist drop warning", () => { + beforeEach(() => { + resetGroupDropWarningsForTests(); + }); + + it("emits a warn log naming the root groups path for the default account", async () => { + const warn = vi.fn(); + const msg = makeUnregisteredGroupMsg("unregistered@g.us"); + const params = makeParams(msg, warn); + + const result = await applyGroupGating(params); + + expect(result).toEqual({ shouldProcess: false }); + expect(warn).toHaveBeenCalledTimes(1); + expect(params.logVerbose).toHaveBeenCalledWith( + 'Dropping message from unregistered WhatsApp group unregistered@g.us. Add the group JID to channels.whatsapp.groups, or add "*" there to admit all groups. Sender authorization still applies.', + ); + const [context, message] = warn.mock.calls[0] ?? []; + expect(context).toMatchObject({ + conversationId: "unregistered@g.us", + accountId: "default", + groupsPath: "channels.whatsapp.groups", + }); + expect(message).toContain("unregistered@g.us"); + expect(message).toContain("channels.whatsapp.groups"); + }); + + it("names the account-scoped groups path for non-default accounts", async () => { + const warn = vi.fn(); + const msg = makeUnregisteredGroupMsg("unregistered@g.us", "work"); + + await applyGroupGating(makeParams(msg, warn)); + + expect(warn).toHaveBeenCalledTimes(1); + const [context, message] = warn.mock.calls[0] ?? []; + expect(context).toMatchObject({ + conversationId: "unregistered@g.us", + accountId: "work", + groupsPath: "channels.whatsapp.accounts.work.groups", + }); + expect(message).toContain("channels.whatsapp.accounts.work.groups"); + }); + + it("names the root groups path for non-default accounts inheriting root groups", async () => { + const warn = vi.fn(); + const msg = makeUnregisteredGroupMsg("unregistered@g.us", "work"); + const cfg = { + channels: { + whatsapp: { + groupPolicy: "allowlist", + groups: { + "registered@g.us": {}, + }, + accounts: { + work: { + groupPolicy: "allowlist", + }, + }, + }, + }, + messages: { + groupChat: { + mentionPatterns: ["\\bopenclaw\\b"], + }, + }, + } as ApplyGroupGatingParams["cfg"]; + + await applyGroupGating(makeParams(msg, warn, cfg)); + + expect(warn).toHaveBeenCalledTimes(1); + const [context, message] = warn.mock.calls[0] ?? []; + expect(context).toMatchObject({ + conversationId: "unregistered@g.us", + accountId: "work", + groupsPath: "channels.whatsapp.groups", + }); + expect(message).toContain("channels.whatsapp.groups"); + }); + + it("warns once but keeps verbose diagnostics per dropped message", async () => { + const warn = vi.fn(); + const first = makeParams(makeUnregisteredGroupMsg("loud@g.us"), warn); + const second = makeParams(makeUnregisteredGroupMsg("loud@g.us"), warn); + const third = makeParams(makeUnregisteredGroupMsg("loud@g.us"), warn); + + await applyGroupGating(first); + await applyGroupGating(second); + await applyGroupGating(third); + + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0]?.[1]).toContain("loud@g.us"); + expect(first.logVerbose).toHaveBeenCalledTimes(1); + expect(second.logVerbose).toHaveBeenCalledTimes(1); + expect(third.logVerbose).toHaveBeenCalledTimes(1); + }); + + it("warns separately for distinct conversations", async () => { + const warn = vi.fn(); + + await applyGroupGating(makeParams(makeUnregisteredGroupMsg("a@g.us"), warn)); + await applyGroupGating(makeParams(makeUnregisteredGroupMsg("b@g.us"), warn)); + + expect(warn).toHaveBeenCalledTimes(2); + expect(warn.mock.calls[0]?.[1]).toContain("a@g.us"); + expect(warn.mock.calls[1]?.[1]).toContain("b@g.us"); + }); + + it("evicts old warning keys instead of growing without bound", async () => { + const warn = vi.fn(); + + await applyGroupGating(makeParams(makeUnregisteredGroupMsg("evicted@g.us"), warn)); + for (let index = 0; index < 100; index += 1) { + await applyGroupGating(makeParams(makeUnregisteredGroupMsg(`overflow-${index}@g.us`), warn)); + } + await applyGroupGating(makeParams(makeUnregisteredGroupMsg("evicted@g.us"), warn)); + + expect(warn).toHaveBeenCalledTimes(102); + expect(warn.mock.calls[0]?.[1]).toContain("evicted@g.us"); + expect(warn.mock.calls[101]?.[1]).toContain("evicted@g.us"); + }); + + it("does not warn when the group is registered", async () => { + const warn = vi.fn(); + const msg = makeUnregisteredGroupMsg("registered@g.us"); + + await applyGroupGating(makeParams(msg, warn)); + + expect(warn).not.toHaveBeenCalled(); + }); +}); diff --git a/extensions/whatsapp/src/auto-reply/monitor/group-gating.audio-preflight.test.ts b/extensions/whatsapp/src/auto-reply/monitor/group-gating.audio-preflight.test.ts index 3bcf5a1845e1..6cf74fdced86 100644 --- a/extensions/whatsapp/src/auto-reply/monitor/group-gating.audio-preflight.test.ts +++ b/extensions/whatsapp/src/auto-reply/monitor/group-gating.audio-preflight.test.ts @@ -49,7 +49,7 @@ function makeParams(msg: WebInboundMsg, groupHistories: Map>(), logVerbose: vi.fn(), - replyLogger: { debug: vi.fn() }, + replyLogger: { debug: vi.fn(), warn: vi.fn() }, }; } diff --git a/extensions/whatsapp/src/auto-reply/monitor/group-gating.ts b/extensions/whatsapp/src/auto-reply/monitor/group-gating.ts index 8b560e914516..3542c48479fc 100644 --- a/extensions/whatsapp/src/auto-reply/monitor/group-gating.ts +++ b/extensions/whatsapp/src/auto-reply/monitor/group-gating.ts @@ -1,4 +1,5 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; +import { resolveWhatsAppGroupsConfigPath } from "../../group-config-path.js"; import { getPrimaryIdentityId, getReplyContext, @@ -46,9 +47,34 @@ type ApplyGroupGatingParams = { groupMemberNames: Map>; selfChatMode?: boolean; logVerbose: (msg: string) => void; - replyLogger: { debug: (obj: unknown, msg: string) => void }; + replyLogger: { + debug: (obj: unknown, msg: string) => void; + warn: (obj: unknown, msg: string) => void; + }; }; +const MAX_GROUP_DROP_WARNINGS = 100; +const groupDropWarned = new Set(); + +export function resetGroupDropWarningsForTests() { + groupDropWarned.clear(); +} + +function shouldWarnForGroupDrop(warnKey: string): boolean { + if (groupDropWarned.has(warnKey)) { + return false; + } + groupDropWarned.add(warnKey); + while (groupDropWarned.size > MAX_GROUP_DROP_WARNINGS) { + const oldest = groupDropWarned.values().next().value; + if (!oldest) { + break; + } + groupDropWarned.delete(oldest); + } + return true; +} + function isOwnerSender(baseMentionConfig: MentionConfig, msg: WebInboundMsg) { const sender = normalizeE164(getSenderIdentity(msg).e164 ?? ""); if (!sender) { @@ -114,6 +140,15 @@ export async function applyGroupGating(params: ApplyGroupGatingParams) { params.conversationId, ); if (conversationGroupPolicy.allowlistEnabled && !conversationGroupPolicy.allowed) { + const accountId = inboundPolicy.account.accountId; + const warnKey = `${accountId}:${params.conversationId}`; + if (shouldWarnForGroupDrop(warnKey)) { + const groupsPath = resolveWhatsAppGroupsConfigPath({ cfg: params.cfg, accountId }); + params.replyLogger.warn( + { conversationId: params.conversationId, accountId, groupsPath }, + `WhatsApp group ${params.conversationId} not in ${groupsPath} — inbound dropped. Add the group JID to ${groupsPath} (or add "*" there to admit all groups). Sender authorization still applies.`, + ); + } params.logVerbose( `Dropping message from unregistered WhatsApp group ${params.conversationId}. Add the group JID to channels.whatsapp.groups, or add "*" there to admit all groups. Sender authorization still applies.`, ); diff --git a/extensions/whatsapp/src/auto-reply/web-auto-reply-monitor.test.ts b/extensions/whatsapp/src/auto-reply/web-auto-reply-monitor.test.ts index b10a72d1fa0d..23300af30644 100644 --- a/extensions/whatsapp/src/auto-reply/web-auto-reply-monitor.test.ts +++ b/extensions/whatsapp/src/auto-reply/web-auto-reply-monitor.test.ts @@ -75,7 +75,7 @@ async function runGroupGating(params: { groupHistoryLimit: 10, groupMemberNames: new Map(), logVerbose: (message) => verboseLogs.push(message), - replyLogger: { debug: () => {} }, + replyLogger: { debug: () => {}, warn: () => {} }, }); return { result, groupHistories, verboseLogs }; } diff --git a/extensions/whatsapp/src/group-config-path.ts b/extensions/whatsapp/src/group-config-path.ts new file mode 100644 index 000000000000..f361f37d9bd9 --- /dev/null +++ b/extensions/whatsapp/src/group-config-path.ts @@ -0,0 +1,98 @@ +import { DEFAULT_ACCOUNT_ID, type OpenClawConfig } from "openclaw/plugin-sdk/account-core"; + +const WHATSAPP_GROUP_SCOPE_FIELDS = ["groupPolicy", "groupAllowFrom", "groups"] as const; + +type WhatsAppGroupScopeField = (typeof WHATSAPP_GROUP_SCOPE_FIELDS)[number]; + +function resolveWhatsAppAccountKey( + accounts: Record | undefined, + accountId: string, +): string | undefined { + if (!accounts) { + return undefined; + } + if (Object.hasOwn(accounts, accountId)) { + return accountId; + } + const normalizedAccountId = accountId.trim().toLowerCase(); + return Object.keys(accounts).find((key) => key.trim().toLowerCase() === normalizedAccountId); +} + +function normalizePathAccountId(accountId?: string | null): string { + return typeof accountId === "string" + ? accountId.trim() || DEFAULT_ACCOUNT_ID + : DEFAULT_ACCOUNT_ID; +} + +function hasConfiguredField(config: unknown, field: WhatsAppGroupScopeField): boolean { + return Boolean( + config && + typeof config === "object" && + Object.hasOwn(config as Record, field) && + (config as Record)[field] !== undefined, + ); +} + +function resolveSpecificFieldBasePath(params: { + cfg: OpenClawConfig; + accountId?: string | null; + field: WhatsAppGroupScopeField; +}): string | undefined { + const accountId = normalizePathAccountId(params.accountId); + const whatsapp = params.cfg.channels?.whatsapp; + const accounts = whatsapp?.accounts as Record | undefined; + const accountKey = resolveWhatsAppAccountKey(accounts, accountId); + const defaultAccountKey = resolveWhatsAppAccountKey(accounts, DEFAULT_ACCOUNT_ID); + const accountConfig = accountKey ? accounts?.[accountKey] : undefined; + const defaultAccountConfig = defaultAccountKey ? accounts?.[defaultAccountKey] : undefined; + if (hasConfiguredField(accountConfig, params.field)) { + return `channels.whatsapp.accounts.${accountKey}`; + } + if (accountId !== DEFAULT_ACCOUNT_ID && hasConfiguredField(defaultAccountConfig, params.field)) { + return `channels.whatsapp.accounts.${defaultAccountKey}`; + } + if (hasConfiguredField(whatsapp, params.field)) { + return "channels.whatsapp"; + } + return undefined; +} + +function resolveWhatsAppGroupScopeBasePath(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): string { + const accountId = normalizePathAccountId(params.accountId); + const whatsapp = params.cfg.channels?.whatsapp; + const accounts = whatsapp?.accounts as Record | undefined; + const accountKey = resolveWhatsAppAccountKey(accounts, accountId); + const defaultAccountKey = resolveWhatsAppAccountKey(accounts, DEFAULT_ACCOUNT_ID); + const accountConfig = accountKey ? accounts?.[accountKey] : undefined; + const defaultAccountConfig = defaultAccountKey ? accounts?.[defaultAccountKey] : undefined; + const matchesAnyGroupScopeField = (config: unknown): boolean => + WHATSAPP_GROUP_SCOPE_FIELDS.some((field) => hasConfiguredField(config, field)); + if (matchesAnyGroupScopeField(accountConfig)) { + return `channels.whatsapp.accounts.${accountKey}`; + } + if (accountId !== DEFAULT_ACCOUNT_ID && matchesAnyGroupScopeField(defaultAccountConfig)) { + return `channels.whatsapp.accounts.${defaultAccountKey}`; + } + return "channels.whatsapp"; +} + +export function resolveWhatsAppConfigPath(params: { + cfg: OpenClawConfig; + accountId?: string | null; + field: WhatsAppGroupScopeField; +}): string { + return `${resolveWhatsAppGroupScopeBasePath(params)}.${params.field}`; +} + +export function resolveWhatsAppGroupsConfigPath(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): string { + return `${ + resolveSpecificFieldBasePath({ ...params, field: "groups" }) ?? + resolveWhatsAppGroupScopeBasePath(params) + }.groups`; +} diff --git a/extensions/whatsapp/src/shared.ts b/extensions/whatsapp/src/shared.ts index f54d4a2c9b56..6de2f069ea8c 100644 --- a/extensions/whatsapp/src/shared.ts +++ b/extensions/whatsapp/src/shared.ts @@ -1,4 +1,3 @@ -import { DEFAULT_ACCOUNT_ID } from "openclaw/plugin-sdk/account-core"; import { describeAccountSnapshot } from "openclaw/plugin-sdk/account-helpers"; import { normalizeE164 } from "openclaw/plugin-sdk/account-resolution"; import { @@ -26,6 +25,7 @@ import { import { formatWhatsAppConfigAllowFromEntries } from "./config-accessors.js"; import { WhatsAppChannelConfigSchema } from "./config-schema.js"; import { whatsappDoctor } from "./doctor.js"; +import { resolveWhatsAppConfigPath } from "./group-config-path.js"; import { resolveLegacyGroupSessionKey } from "./group-session-contract.js"; import { collectUnsupportedSecretRefConfigCandidates, @@ -40,56 +40,6 @@ import { const WHATSAPP_CHANNEL = "whatsapp" as const; -const WHATSAPP_GROUP_SCOPE_FIELDS = ["groupPolicy", "groupAllowFrom", "groups"] as const; - -type WhatsAppGroupScopeField = (typeof WHATSAPP_GROUP_SCOPE_FIELDS)[number]; - -function resolveWhatsAppAccountKey( - accounts: Record | undefined, - accountId: string, -): string | undefined { - if (!accounts) { - return undefined; - } - if (Object.hasOwn(accounts, accountId)) { - return accountId; - } - const normalizedAccountId = accountId.trim().toLowerCase(); - return Object.keys(accounts).find((key) => key.trim().toLowerCase() === normalizedAccountId); -} - -function resolveWhatsAppGroupScopeBasePath(params: { - cfg: Parameters[0]["cfg"]; - accountId?: string | null; -}): string { - const accountId = - typeof params.accountId === "string" - ? params.accountId.trim() || DEFAULT_ACCOUNT_ID - : DEFAULT_ACCOUNT_ID; - const accounts = params.cfg.channels?.whatsapp?.accounts; - const accountKey = resolveWhatsAppAccountKey(accounts, accountId); - const defaultAccountKey = resolveWhatsAppAccountKey(accounts, DEFAULT_ACCOUNT_ID); - const accountConfig = accountKey ? accounts?.[accountKey] : undefined; - const defaultAccountConfig = defaultAccountKey ? accounts?.[defaultAccountKey] : undefined; - const matchesAnyGroupScopeField = (config: Record | undefined): boolean => - WHATSAPP_GROUP_SCOPE_FIELDS.some((field) => config?.[field] !== undefined); - if (matchesAnyGroupScopeField(accountConfig)) { - return `channels.whatsapp.accounts.${accountKey}`; - } - if (accountId !== DEFAULT_ACCOUNT_ID && matchesAnyGroupScopeField(defaultAccountConfig)) { - return `channels.whatsapp.accounts.${defaultAccountKey}`; - } - return "channels.whatsapp"; -} - -function resolveWhatsAppConfigPath(params: { - cfg: Parameters[0]["cfg"]; - accountId?: string | null; - field: WhatsAppGroupScopeField; -}): string { - return `${resolveWhatsAppGroupScopeBasePath(params)}.${params.field}`; -} - export async function loadWhatsAppChannelRuntime() { return await import("./channel.runtime.js"); }