From ac99e907a1a9d28583dfbcb0cadee36c7e68f181 Mon Sep 17 00:00:00 2001 From: gavrielc Date: Fri, 10 Jul 2026 21:55:41 +0300 Subject: [PATCH] PR8: make WhatsApp adapter shared-number aware, drop core config imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Adapter reads ASSISTANT_NAME / ASSISTANT_HAS_OWN_NUMBER from .env itself (readEnvFile at module load); shared = value !== 'true', exported as resolveSharedMode for the truth-table test - computeIsMention gains the shared flag and returns undefined for everything in shared mode (DMs address the human; owner tags tag the human), killing the approval-card spam at the source; dedicated truth table unchanged - botLid → @ASSISTANT_NAME content rewrite extracted to rewriteBotLidMention and skipped in shared mode so friend tags of the owner can't false-fire name-pattern wirings; outbound ': ' prefix and isBotMessage now key off the same flag (behaviorally identical to !ASSISTANT_HAS_OWN_NUMBER) - Once-per-chatJid debug log when shared mode forwards a non-self-chat message without isMention, for /debug discoverability Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PJkDAnLGWJUqjgJNXpyqGZ --- src/channels/whatsapp.test.ts | 116 +++++++++++++++++++++++++++------- src/channels/whatsapp.ts | 112 ++++++++++++++++++++++++-------- 2 files changed, 179 insertions(+), 49 deletions(-) diff --git a/src/channels/whatsapp.test.ts b/src/channels/whatsapp.test.ts index 95a17245d..9afc2a7c0 100644 --- a/src/channels/whatsapp.test.ts +++ b/src/channels/whatsapp.test.ts @@ -1,26 +1,26 @@ /** * Regression coverage for #2560 — group @-mentions of the bot must set - * `InboundMessage.isMention`. Before the fix, the inbound construction - * site hard-coded `isMention: !isGroup ? true : undefined`, which dropped - * every group mention on the floor and prevented the router from waking - * the agent on a mention-only trigger. + * `InboundMessage.isMention` — plus the shared-number mode split: when + * ASSISTANT_HAS_OWN_NUMBER is not explicitly 'true' the adapter runs on the + * operator's personal number, so NOTHING is a bot mention (DMs address the + * human; group tags of the owner tag the human) and the bot-LID → assistant + * name content rewrite is skipped. * * The detection logic lives in the exported pure helper `isBotMentionedInGroup`; - * the inbound site calls it with `normalized`, `botPhoneJid`, `botLidUser`. - * `isMention` is then computed as: - * - * isMention: !isGroup ? true : botMentionedInGroup ? true : undefined - * - * Both the helper and the call-site ternary are covered below so a future - * refactor that breaks either part fails this suite. + * the inbound site feeds it into `computeIsMention(shared, isGroup, mentioned)`. + * Both helpers and the mode-resolution truth table are covered below so a + * future refactor that breaks any part fails this suite. */ import { describe, it, expect } from 'vitest'; import { appendMediaFailureNote, computeIsMention, + computeWhatsappDefaults, isBotMentionedInGroup, parseWhatsAppMentions, + resolveSharedMode, + rewriteBotLidMention, } from './whatsapp.js'; const BOT_PHONE_JID = '15550009999@s.whatsapp.net'; @@ -89,19 +89,91 @@ describe('isBotMentionedInGroup (#2560)', () => { }); }); -describe('InboundMessage.isMention semantics (#2560)', () => { - it('is undefined for a group message with no bot mention', () => { - expect(computeIsMention(true, false)).toBeUndefined(); +describe('InboundMessage.isMention semantics (#2560 + shared mode)', () => { + describe('dedicated mode (bot has its own number)', () => { + it('is undefined for a group message with no bot mention', () => { + expect(computeIsMention(false, true, false)).toBeUndefined(); + }); + + it('is true for a group message where the bot is mentioned', () => { + expect(computeIsMention(false, true, true)).toBe(true); + }); + + it('is true for a DM regardless of mention state', () => { + // DMs are unconditionally mentions — the helper isn't consulted there. + expect(computeIsMention(false, false, false)).toBe(true); + expect(computeIsMention(false, false, true)).toBe(true); + }); }); - it('is true for a group message where the bot is mentioned', () => { - expect(computeIsMention(true, true)).toBe(true); + describe('shared mode (operator personal number)', () => { + it('is undefined for EVERYTHING — DMs address the human, tags tag the human', () => { + expect(computeIsMention(true, false, false)).toBeUndefined(); + expect(computeIsMention(true, false, true)).toBeUndefined(); + expect(computeIsMention(true, true, false)).toBeUndefined(); + expect(computeIsMention(true, true, true)).toBeUndefined(); + }); + }); +}); + +describe('resolveSharedMode env truth table', () => { + it('explicit "true" → dedicated (not shared)', () => { + expect(resolveSharedMode('true')).toBe(false); }); - it('is true for a DM regardless of mention state', () => { - // DMs are unconditionally mentions — the helper isn't consulted there. - expect(computeIsMention(false, false)).toBe(true); - expect(computeIsMention(false, true)).toBe(true); + it('absent or any other value → shared', () => { + expect(resolveSharedMode(undefined)).toBe(true); + expect(resolveSharedMode('')).toBe(true); + expect(resolveSharedMode('false')).toBe(true); + expect(resolveSharedMode('TRUE')).toBe(true); + expect(resolveSharedMode('1')).toBe(true); + expect(resolveSharedMode('yes')).toBe(true); + }); +}); + +describe('rewriteBotLidMention', () => { + const ASSISTANT = 'Andy'; + + it('dedicated mode rewrites a bot-LID tag into @', () => { + expect(rewriteBotLidMention(`hey @${BOT_LID_USER} status?`, false, BOT_LID_USER, ASSISTANT)).toBe( + 'hey @Andy status?', + ); + }); + + it('shared mode skips the rewrite — a tag of the owner must not become name-pattern bait', () => { + const content = `hey @${BOT_LID_USER} status?`; + expect(rewriteBotLidMention(content, true, BOT_LID_USER, ASSISTANT)).toBe(content); + }); + + it('passes content through when the bot LID is unknown', () => { + expect(rewriteBotLidMention(`hey @${BOT_LID_USER}`, false, undefined, ASSISTANT)).toBe(`hey @${BOT_LID_USER}`); + }); + + it('passes content through when there is no tag of the bot LID', () => { + expect(rewriteBotLidMention('no tags here', false, BOT_LID_USER, ASSISTANT)).toBe('no tags here'); + }); +}); + +describe('computeWhatsappDefaults', () => { + it('shared mode declares strict name-pattern defaults with no platform mentions', () => { + expect(computeWhatsappDefaults(true)).toEqual({ + dm: { engageMode: 'pattern', engagePattern: '.', threads: false, unknownSenderPolicy: 'strict' }, + group: { + engageMode: 'pattern', + engagePattern: '\\b{name}\\b', + threads: false, + unknownSenderPolicy: 'strict', + }, + mentions: 'never', + }); + }); + + it('dedicated mode declares platform mentions with group engage on mention (never sticky)', () => { + expect(computeWhatsappDefaults(false)).toEqual({ + dm: { engageMode: 'pattern', engagePattern: '.', threads: false, unknownSenderPolicy: 'request_approval' }, + group: { engageMode: 'mention', threads: false, unknownSenderPolicy: 'request_approval' }, + mentions: 'platform', + }); }); }); @@ -172,9 +244,7 @@ describe('appendMediaFailureNote', () => { }); it('appends the note on its own line when a captioned message has a failed download', () => { - expect(appendMediaFailureNote('check this out', ['image'])).toBe( - 'check this out\n[image could not be downloaded]', - ); + expect(appendMediaFailureNote('check this out', ['image'])).toBe('check this out\n[image could not be downloaded]'); }); it('uses the note as the content when an uncaptioned media message fails (would otherwise be dropped)', () => { diff --git a/src/channels/whatsapp.ts b/src/channels/whatsapp.ts index 9d597a71c..4bd61a15c 100644 --- a/src/channels/whatsapp.ts +++ b/src/channels/whatsapp.ts @@ -39,7 +39,7 @@ import { import type { GroupMetadata, WAMessageKey, WAMessage, WASocket } from '@whiskeysockets/baileys'; import { isSafeAttachmentName } from '../attachment-safety.js'; -import { ASSISTANT_HAS_OWN_NUMBER, ASSISTANT_NAME, DATA_DIR } from '../config.js'; +import { DATA_DIR } from '../config.js'; import { readEnvFile } from '../env.js'; import { log } from '../log.js'; import { registerChannelAdapter } from './channel-registry.js'; @@ -240,7 +240,14 @@ export function isBotMentionedInGroup( } /** - * Compute `InboundMessage.isMention` for a WhatsApp message: + * Compute `InboundMessage.isMention` for a WhatsApp message. + * + * Shared-number mode (operator's personal number): NOTHING is a mention. + * DMs are addressed to the human, and a group tag of the owner's JID/LID + * tags the human — treating either as a bot mention would auto-create + * messaging groups and fire approval cards for ordinary human traffic. + * + * Dedicated mode (bot has its own number): * - DMs are always mentions (router auto-engages on the bot's behalf). * - Group messages are mentions only when the bot is explicitly tagged. * @@ -248,11 +255,29 @@ export function isBotMentionedInGroup( * `InboundMessage` field is `isMention?: boolean` and downstream code * treats `undefined` differently than an explicit `false` (#2560). */ -export function computeIsMention(isGroup: boolean, botMentionedInGroup: boolean): true | undefined { +export function computeIsMention(shared: boolean, isGroup: boolean, botMentionedInGroup: boolean): true | undefined { + if (shared) return undefined; if (!isGroup) return true; return botMentionedInGroup ? true : undefined; } +/** + * Normalize a tag of the bot's LID into `@` so mention text + * matches name-pattern triggers. Dedicated mode only: on a shared number the + * LID belongs to the human owner, and rewriting a friend's tag of the owner + * into the assistant's name would make name-pattern wirings false-fire on + * every such tag. + */ +export function rewriteBotLidMention( + content: string, + shared: boolean, + botLidUser: string | undefined, + assistantName: string, +): string { + if (shared || !botLidUser || !content.includes(`@${botLidUser}`)) return content; + return content.replace(`@${botLidUser}`, `@${assistantName}`); +} + /** * Append a visible note for media that failed to download, so the agent knows * something was sent rather than silently losing the attachment — or the whole @@ -285,6 +310,16 @@ function buildMediaMessage(data: Buffer, filename: string, ext: string, caption? return { document: data, fileName: filename, caption, mimetype: 'application/octet-stream' }; } +/** + * Shared vs dedicated number mode. Only an explicit ASSISTANT_HAS_OWN_NUMBER=true + * means the bot has its own number (dedicated); anything else — absent, empty, + * 'false', any other string — means the bot rides the operator's personal + * number (shared). Exported for unit testing the truth table. + */ +export function resolveSharedMode(assistantHasOwnNumber: string | undefined): boolean { + return assistantHasOwnNumber !== 'true'; +} + /** * Shared vs dedicated number changes every default, so the declaration is * computed once at module load from the adapter's own env: @@ -295,19 +330,30 @@ function buildMediaMessage(data: Buffer, filename: string, ext: string, caption? * - dedicated: a real bot number. Groups engage on platform mentions — * 'mention', NEVER 'mention-sticky': WhatsApp is non-threaded and sessions * are never deleted, so sticky would mean engaged-forever. + * + * Exported for unit testing both mode literals. */ -const WHATSAPP_SHARED = readEnvFile(['ASSISTANT_HAS_OWN_NUMBER']).ASSISTANT_HAS_OWN_NUMBER !== 'true'; -const WHATSAPP_DEFAULTS: ChannelDefaults = WHATSAPP_SHARED - ? { - dm: { engageMode: 'pattern', engagePattern: '.', threads: false, unknownSenderPolicy: 'strict' }, - group: { engageMode: 'pattern', engagePattern: '\\b{name}\\b', threads: false, unknownSenderPolicy: 'strict' }, - mentions: 'never', - } - : { - dm: { engageMode: 'pattern', engagePattern: '.', threads: false, unknownSenderPolicy: 'request_approval' }, - group: { engageMode: 'mention', threads: false, unknownSenderPolicy: 'request_approval' }, - mentions: 'platform', - }; +export function computeWhatsappDefaults(shared: boolean): ChannelDefaults { + return shared + ? { + dm: { engageMode: 'pattern', engagePattern: '.', threads: false, unknownSenderPolicy: 'strict' }, + group: { engageMode: 'pattern', engagePattern: '\\b{name}\\b', threads: false, unknownSenderPolicy: 'strict' }, + mentions: 'never', + } + : { + dm: { engageMode: 'pattern', engagePattern: '.', threads: false, unknownSenderPolicy: 'request_approval' }, + group: { engageMode: 'mention', threads: false, unknownSenderPolicy: 'request_approval' }, + mentions: 'platform', + }; +} + +// Adapter-internal env: same .env keys as always (setup/channels/whatsapp.ts +// still writes them), but read here instead of imported from core config — +// shared-number handling is channel-local. +const waEnv = readEnvFile(['ASSISTANT_NAME', 'ASSISTANT_HAS_OWN_NUMBER']); +const ASSISTANT_NAME = waEnv.ASSISTANT_NAME || 'Andy'; +const WHATSAPP_SHARED = resolveSharedMode(waEnv.ASSISTANT_HAS_OWN_NUMBER); +const WHATSAPP_DEFAULTS: ChannelDefaults = computeWhatsappDefaults(WHATSAPP_SHARED); registerChannelAdapter('whatsapp', { factory: () => { @@ -356,6 +402,9 @@ registerChannelAdapter('whatsapp', { let lastGroupSync = 0; let groupSyncTimerStarted = false; + // Chats already noted in the shared-mode once-per-chat debug log + const sharedModeLoggedChats = new Set(); + // First-connect promise let resolveFirstOpen: (() => void) | undefined; let rejectFirstOpen: ((err: Error) => void) | undefined; @@ -751,9 +800,8 @@ registerChannelAdapter('whatsapp', { ''; // Normalize bot LID mention → assistant name for trigger matching - if (botLidUser && content.includes(`@${botLidUser}`)) { - content = content.replace(`@${botLidUser}`, `@${ASSISTANT_NAME}`); - } + // (dedicated mode only — see rewriteBotLidMention) + content = rewriteBotLidMention(content, WHATSAPP_SHARED, botLidUser, ASSISTANT_NAME); // Download media attachments (images, video, audio, documents) const { attachments, failures } = await downloadInboundMedia(msg, normalized); @@ -785,7 +833,7 @@ registerChannelAdapter('whatsapp', { if (sentMessageCache.has(msg.key.id || '')) continue; } - const isBotMessage = ASSISTANT_HAS_OWN_NUMBER ? false : content.startsWith(`${ASSISTANT_NAME}:`); + const isBotMessage = WHATSAPP_SHARED ? content.startsWith(`${ASSISTANT_NAME}:`) : false; // Check if this reply answers a pending question via slash command const pending = pendingQuestions.get(chatJid); @@ -815,12 +863,13 @@ registerChannelAdapter('whatsapp', { const inbound: InboundMessage = { id: msg.key.id || `wa-${Date.now()}`, kind: 'chat', - // DMs are addressed to the bot by definition. Mark them as - // platform-confirmed mentions so the router auto-creates an - // approval-required messaging_group when the chat is unknown, - // instead of silently dropping. In groups, only an explicit - // @-mention counts. - isMention: computeIsMention(isGroup, botMentionedInGroup), + // Dedicated mode: DMs are addressed to the bot by definition. + // Mark them as platform-confirmed mentions so the router + // auto-creates an approval-required messaging_group when the + // chat is unknown, instead of silently dropping. In groups, + // only an explicit @-mention counts. Shared mode: never a + // mention — DMs and tags address the human owner. + isMention: computeIsMention(WHATSAPP_SHARED, isGroup, botMentionedInGroup), isGroup, content: { text: content, @@ -835,6 +884,17 @@ registerChannelAdapter('whatsapp', { timestamp, }; + // Discoverability for /debug: in shared mode nothing carries + // isMention, so unknown chats never auto-create messaging groups + // — traffic can look silently dropped. Note each chat once. + if (WHATSAPP_SHARED && chatJid !== botPhoneJid && !sharedModeLoggedChats.has(chatJid)) { + sharedModeLoggedChats.add(chatJid); + log.debug('Shared-number mode: forwarding chat to router without isMention', { + chatJid, + isGroup, + }); + } + // WhatsApp doesn't use threads — threadId is null setupConfig.onInbound(chatJid, null, inbound); } catch (err) { @@ -949,7 +1009,7 @@ registerChannelAdapter('whatsapp', { if (text) { const { text: formatted, mentions } = formatWhatsApp(text); - const prefixed = ASSISTANT_HAS_OWN_NUMBER ? formatted : `${ASSISTANT_NAME}: ${formatted}`; + const prefixed = WHATSAPP_SHARED ? `${ASSISTANT_NAME}: ${formatted}` : formatted; return sendRawMessage(platformId, prefixed, mentions); } },