diff --git a/extensions/bluebubbles/src/monitor-processing-chat-resolve.test.ts b/extensions/bluebubbles/src/monitor-processing-chat-resolve.test.ts new file mode 100644 index 00000000000..ecd5e57283c --- /dev/null +++ b/extensions/bluebubbles/src/monitor-processing-chat-resolve.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, it } from "vitest"; +import { buildBlueBubblesInboundChatResolveTarget } from "./monitor-processing.js"; + +describe("buildBlueBubblesInboundChatResolveTarget", () => { + it("uses chat_id for group inbound when chatId is present", () => { + const target = buildBlueBubblesInboundChatResolveTarget({ + isGroup: true, + chatId: 42, + chatIdentifier: undefined, + senderId: "+15551234567", + }); + expect(target).toEqual({ kind: "chat_id", chatId: 42 }); + }); + + it("uses chat_identifier for group inbound when chatId missing but identifier present", () => { + const target = buildBlueBubblesInboundChatResolveTarget({ + isGroup: true, + chatId: undefined, + chatIdentifier: "iMessage;+;chat-abc", + senderId: "+15551234567", + }); + expect(target).toEqual({ + kind: "chat_identifier", + chatIdentifier: "iMessage;+;chat-abc", + }); + }); + + it("prefers chat_id over chat_identifier when both are present for a group", () => { + const target = buildBlueBubblesInboundChatResolveTarget({ + isGroup: true, + chatId: 7, + chatIdentifier: "iMessage;+;chat-abc", + senderId: "+15551234567", + }); + expect(target).toEqual({ kind: "chat_id", chatId: 7 }); + }); + + it("REFUSES sender-handle fallback for group inbound with no chat identifiers", () => { + // This is the candidate-4 regression: BlueBubbles webhooks for tapbacks + // and certain reaction/updated-message events arrive without chatGuid/ + // chatId/chatIdentifier. Falling through to { kind: "handle", + // address: senderId } would resolve the sender's DM chatGuid and + // poison every action keyed off it (ack reaction, mark-read, outbound + // reply cache), making group reactions land in DMs. + const target = buildBlueBubblesInboundChatResolveTarget({ + isGroup: true, + chatId: undefined, + chatIdentifier: undefined, + senderId: "+15551234567", + }); + expect(target).toBeNull(); + }); + + it("treats blank chatIdentifier as missing for group inbound", () => { + const target = buildBlueBubblesInboundChatResolveTarget({ + isGroup: true, + chatId: undefined, + chatIdentifier: " ", + senderId: "+15551234567", + }); + expect(target).toBeNull(); + }); + + it("treats non-finite chatId as missing for group inbound", () => { + const target = buildBlueBubblesInboundChatResolveTarget({ + isGroup: true, + chatId: Number.NaN, + chatIdentifier: undefined, + senderId: "+15551234567", + }); + expect(target).toBeNull(); + }); + + it("treats null chatId/chatIdentifier as missing for group inbound", () => { + const target = buildBlueBubblesInboundChatResolveTarget({ + isGroup: true, + chatId: null, + chatIdentifier: null, + senderId: "+15551234567", + }); + expect(target).toBeNull(); + }); + + it("uses sender handle for DM inbound (the chat IS the conversation with that sender)", () => { + const target = buildBlueBubblesInboundChatResolveTarget({ + isGroup: false, + chatId: undefined, + chatIdentifier: undefined, + senderId: "+15551234567", + }); + expect(target).toEqual({ kind: "handle", address: "+15551234567" }); + }); + + it("uses sender handle for DM inbound even when chatId is present (preserves prior behavior)", () => { + const target = buildBlueBubblesInboundChatResolveTarget({ + isGroup: false, + chatId: 99, + chatIdentifier: "iMessage;-;+15551234567", + senderId: "+15551234567", + }); + expect(target).toEqual({ kind: "handle", address: "+15551234567" }); + }); + + it("returns null for DM inbound with empty senderId", () => { + const target = buildBlueBubblesInboundChatResolveTarget({ + isGroup: false, + chatId: undefined, + chatIdentifier: undefined, + senderId: " ", + }); + expect(target).toBeNull(); + }); +}); diff --git a/extensions/bluebubbles/src/monitor-processing.ts b/extensions/bluebubbles/src/monitor-processing.ts index c36dcf3851b..d2b98befdaa 100644 --- a/extensions/bluebubbles/src/monitor-processing.ts +++ b/extensions/bluebubbles/src/monitor-processing.ts @@ -354,6 +354,52 @@ export function logVerbose( } } +export type BlueBubblesInboundChatResolveTarget = + | { readonly kind: "chat_id"; readonly chatId: number } + | { readonly kind: "chat_identifier"; readonly chatIdentifier: string } + | { readonly kind: "handle"; readonly address: string }; + +/** + * Builds the fallback target used to look up a chatGuid when an inbound + * webhook arrives without one. + * + * Critically, group inbounds that lack every chat identifier (chatGuid / + * chatId / chatIdentifier all missing) MUST NOT fall through to the + * sender's handle. Resolving a group via the sender handle yields that + * sender's DM chatGuid, which then poisons every downstream action keyed + * off it: ack reactions land in the DM, the read receipt marks the DM, + * and the outbound reply cache stores the wrong chat — so a later short + * id resolved against that cache cannot detect the cross-chat reuse and + * the agent's react/reply silently target the DM instead of the group. + * + * Returns null in that unresolvable group case so the caller can skip + * actions that need a chatGuid rather than acting on a wrong one. DMs + * always resolve via the sender handle (the chat is, by definition, the + * conversation with that handle). + */ +export function buildBlueBubblesInboundChatResolveTarget(params: { + isGroup: boolean; + chatId?: number | null; + chatIdentifier?: string | null; + senderId: string; +}): BlueBubblesInboundChatResolveTarget | null { + if (params.isGroup) { + if (typeof params.chatId === "number" && Number.isFinite(params.chatId)) { + return { kind: "chat_id", chatId: params.chatId }; + } + const trimmedIdentifier = params.chatIdentifier?.trim(); + if (trimmedIdentifier) { + return { kind: "chat_identifier", chatIdentifier: trimmedIdentifier }; + } + return null; + } + const trimmedSender = params.senderId.trim(); + if (!trimmedSender) { + return null; + } + return { kind: "handle", address: trimmedSender }; +} + function logGroupAllowlistHint(params: { runtime: BlueBubblesRuntimeEnv; reason: string; @@ -1295,13 +1341,13 @@ async function processMessageAfterDedupe( }); let chatGuidForActions = chatGuid; if (!chatGuidForActions && baseUrl && password) { - const resolveTarget = - isGroup && (chatId || chatIdentifier) - ? chatId - ? ({ kind: "chat_id", chatId } as const) - : ({ kind: "chat_identifier", chatIdentifier: chatIdentifier ?? "" } as const) - : ({ kind: "handle", address: message.senderId } as const); - if (resolveTarget.kind !== "chat_identifier" || resolveTarget.chatIdentifier) { + const resolveTarget = buildBlueBubblesInboundChatResolveTarget({ + isGroup, + chatId, + chatIdentifier, + senderId: message.senderId, + }); + if (resolveTarget) { chatGuidForActions = (await resolveChatGuidForTarget({ baseUrl, @@ -1309,6 +1355,12 @@ async function processMessageAfterDedupe( target: resolveTarget, allowPrivateNetwork: isPrivateNetworkOptInEnabled(account.config), })) ?? undefined; + } else { + logVerbose( + core, + runtime, + `cannot resolve chatGuid for group inbound (chatGuid/chatId/chatIdentifier all missing); senderId=${message.senderId}`, + ); } }