diff --git a/extensions/signal/src/monitor/event-handler.emoji-reactions.test.ts b/extensions/signal/src/monitor/event-handler.emoji-reactions.test.ts new file mode 100644 index 00000000000..690e7a2ae38 --- /dev/null +++ b/extensions/signal/src/monitor/event-handler.emoji-reactions.test.ts @@ -0,0 +1,247 @@ +/** + * Tests for Signal emoji reaction handling. + * + * When a remote user reacts to a message with an emoji (👍, ❤️, etc.), + * signal-cli sends a dataMessage with a `reaction` field. These should be + * surfaced as system events to the agent session (matching Discord's pattern) + * rather than leaking through as . + * + * Two paths: + * 1. Well-formed reactions (isSignalReactionMessage returns true) → + * handled by existing handleReactionOnlyInbound + * 2. Bare/malformed reactions (isSignalReactionMessage returns false, + * e.g. targetAuthor absent) → new hasBareReactionField guard surfaces + * as system event + */ + +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { createSignalEventHandler } from "./event-handler.js"; +import { + createBaseSignalEventHandlerDeps, + createSignalReceiveEvent, +} from "./event-handler.test-harness.js"; + +const { dispatchInboundMessageMock, enqueueSystemEventMock } = vi.hoisted(() => ({ + dispatchInboundMessageMock: vi.fn().mockResolvedValue({ + queuedFinal: false, + counts: { tool: 0, block: 0, final: 0 }, + }), + enqueueSystemEventMock: vi.fn(), +})); + +vi.mock("../send.js", () => ({ + sendMessageSignal: vi.fn(), + sendTypingSignal: vi.fn().mockResolvedValue(true), + sendReadReceiptSignal: vi.fn().mockResolvedValue(true), +})); + +vi.mock("../../../../src/auto-reply/dispatch.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + dispatchInboundMessage: dispatchInboundMessageMock, + dispatchInboundMessageWithDispatcher: dispatchInboundMessageMock, + dispatchInboundMessageWithBufferedDispatcher: dispatchInboundMessageMock, + }; +}); + +vi.mock("../../../../src/infra/system-events.js", () => ({ + enqueueSystemEvent: enqueueSystemEventMock, +})); + +vi.mock("../../../../src/pairing/pairing-store.js", () => ({ + readChannelAllowFromStore: vi.fn().mockResolvedValue([]), + upsertChannelPairingRequest: vi.fn(), +})); + +describe("signal createSignalEventHandler emoji reaction handling", () => { + beforeEach(() => { + dispatchInboundMessageMock.mockClear(); + enqueueSystemEventMock.mockClear(); + }); + + it("drops a well-formed reaction envelope via handleReactionOnlyInbound", async () => { + const deps = createBaseSignalEventHandlerDeps({ + isSignalReactionMessage: (r): r is NonNullable => + Boolean(r?.emoji && r?.targetSentTimestamp && (r?.targetAuthor || r?.targetAuthorUuid)), + }); + const handler = createSignalEventHandler(deps); + + await handler( + createSignalReceiveEvent({ + dataMessage: { + timestamp: 1700000000000, + message: "", + reaction: { + emoji: "👍", + isRemove: false, + targetAuthor: "+15550001111", + targetSentTimestamp: 1699999000000, + }, + }, + }), + ); + + expect(dispatchInboundMessageMock).not.toHaveBeenCalled(); + }); + + it("surfaces a bare reaction (missing targetAuthor) as a system event", async () => { + // isSignalReactionMessage returns false (default harness) → bare reaction guard kicks in + const deps = createBaseSignalEventHandlerDeps({ + reactionMode: "all", + shouldEmitSignalReactionNotification: () => true, + buildSignalReactionSystemEventText: (params) => + `Signal reaction added: ${params.emojiLabel} by ${params.actorLabel} msg ${params.messageId}`, + }); + const handler = createSignalEventHandler(deps); + + await handler( + createSignalReceiveEvent({ + sourceName: "Alice", + dataMessage: { + timestamp: 1700000000000, + message: "", + reaction: { + emoji: "👍", + isRemove: false, + // targetAuthor / targetAuthorUuid deliberately absent + targetSentTimestamp: 1699999000000, + }, + }, + }), + ); + + expect(dispatchInboundMessageMock).not.toHaveBeenCalled(); + expect(enqueueSystemEventMock).toHaveBeenCalledTimes(1); + expect(enqueueSystemEventMock).toHaveBeenCalledWith( + expect.stringContaining("👍"), + expect.objectContaining({ + contextKey: expect.stringContaining("reaction"), + }), + ); + }); + + it("includes targetSentTimestamp in system event text when available", async () => { + const capturedText: string[] = []; + const deps = createBaseSignalEventHandlerDeps({ + reactionMode: "all", + shouldEmitSignalReactionNotification: () => true, + buildSignalReactionSystemEventText: (params) => { + const text = `Signal reaction added: ${params.emojiLabel} by ${params.actorLabel} msg ${params.messageId}`; + capturedText.push(text); + return text; + }, + }); + const handler = createSignalEventHandler(deps); + + await handler( + createSignalReceiveEvent({ + sourceName: "Alice", + dataMessage: { + timestamp: 1700000000000, + message: "", + reaction: { + emoji: "❤️", + isRemove: false, + targetSentTimestamp: 1699999000000, + }, + }, + }), + ); + + expect(capturedText[0]).toContain("1699999000000"); + }); + + it("drops a bare reaction-removal (isRemove: true) silently", async () => { + const deps = createBaseSignalEventHandlerDeps(); + const handler = createSignalEventHandler(deps); + + await handler( + createSignalReceiveEvent({ + dataMessage: { + timestamp: 1700000000000, + message: "", + reaction: { + emoji: "👍", + isRemove: true, + targetSentTimestamp: 1699999000000, + }, + }, + }), + ); + + expect(dispatchInboundMessageMock).not.toHaveBeenCalled(); + expect(enqueueSystemEventMock).not.toHaveBeenCalled(); + }); + + it("handles a bare reaction that arrives with a null-contentType attachment (signal-cli thumbnail)", async () => { + const deps = createBaseSignalEventHandlerDeps({ + reactionMode: "all", + shouldEmitSignalReactionNotification: () => true, + buildSignalReactionSystemEventText: (params) => + `Signal reaction added: ${params.emojiLabel} by ${params.actorLabel}`, + }); + const handler = createSignalEventHandler(deps); + + await handler( + createSignalReceiveEvent({ + sourceName: "Alice", + dataMessage: { + timestamp: 1700000000000, + message: "", + reaction: { + emoji: "👍", + isRemove: false, + targetSentTimestamp: 1699999000000, + }, + attachments: [{ id: "thumb1", contentType: null, size: 0 }], + }, + }), + ); + + expect(dispatchInboundMessageMock).not.toHaveBeenCalled(); + expect(enqueueSystemEventMock).toHaveBeenCalledTimes(1); + expect(enqueueSystemEventMock).toHaveBeenCalledWith( + expect.stringContaining("👍"), + expect.objectContaining({ contextKey: expect.stringContaining("reaction") }), + ); + }); + + it("does NOT intercept a reaction envelope that also has message text", async () => { + const deps = createBaseSignalEventHandlerDeps(); + const handler = createSignalEventHandler(deps); + + await handler( + createSignalReceiveEvent({ + dataMessage: { + timestamp: 1700000000000, + message: "hello", + reaction: { + emoji: "👍", + isRemove: false, + targetSentTimestamp: 1699999000000, + }, + }, + }), + ); + + // Message has body text — should be dispatched normally + expect(dispatchInboundMessageMock).toHaveBeenCalledTimes(1); + }); + + it("does NOT intercept a plain message without a reaction field", async () => { + const deps = createBaseSignalEventHandlerDeps(); + const handler = createSignalEventHandler(deps); + + await handler( + createSignalReceiveEvent({ + dataMessage: { + timestamp: 1700000000000, + message: "hey there", + }, + }), + ); + + expect(dispatchInboundMessageMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/extensions/signal/src/monitor/event-handler.ts b/extensions/signal/src/monitor/event-handler.ts index 58ff8d4f8d7..a19fd46f5a1 100644 --- a/extensions/signal/src/monitor/event-handler.ts +++ b/extensions/signal/src/monitor/event-handler.ts @@ -521,9 +521,17 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { const messageText = normalizedMessage.trim(); const quoteText = dataMessage?.quote?.text?.trim() ?? ""; - const hasBodyContent = - Boolean(messageText || quoteText) || Boolean(!reaction && dataMessage?.attachments?.length); - const senderDisplay = formatSignalSenderDisplay(sender); + + // Guard: if dataMessage carries a reaction field but isSignalReactionMessage() + // returned null (e.g. targetAuthor/targetAuthorUuid absent in some signal-cli versions), + // and there is no real body content, surface it as a system event (matching the + // Discord reaction pattern) rather than leaking through as . + const bareReaction = dataMessage?.reaction; + // Note: some signal-cli builds attach a null-contentType attachment alongside + // the reaction field (e.g. thumbnail of the reacted-to message). We intentionally + // omit the attachments check so those are caught here rather than leaking as + // . + // Resolve full access state early — shared by bare reaction path and normal dispatch. const { resolveAccessDecision, dmAccess, effectiveDmAllow, effectiveGroupAllow } = await resolveSignalAccessState({ accountId: deps.accountId, @@ -534,6 +542,90 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { sender, }); + const hasBareReactionField = !reaction && Boolean(bareReaction) && !messageText && !quoteText; + if (hasBareReactionField && bareReaction) { + const senderDisplayBare = formatSignalSenderDisplay(sender); + const emojiLabel = (bareReaction as { emoji?: string | null }).emoji?.trim() || "emoji"; + const isRemove = Boolean((bareReaction as { isRemove?: boolean | null }).isRemove); + const targetTimestamp = (bareReaction as { targetSentTimestamp?: number | null }) + .targetSentTimestamp; + logVerbose(`signal: bare reaction (${emojiLabel}) from ${senderDisplayBare}`); + if (!isRemove) { + // P2: per-field fallback so a present-but-empty reaction.groupInfo doesn't shadow + // a populated dataMessage.groupInfo (e.g. Signal emits groupInfo={} on the reaction + // envelope while the real groupId/groupName live on dataMessage.groupInfo). + const bareReactionGroupInfo = bareReaction as { + groupInfo?: { groupId?: string; groupName?: string } | null; + }; + const groupId = + bareReactionGroupInfo.groupInfo?.groupId ?? dataMessage?.groupInfo?.groupId ?? undefined; + const groupName = + bareReactionGroupInfo.groupInfo?.groupName ?? + dataMessage?.groupInfo?.groupName ?? + undefined; + const isGroup = Boolean(groupId); + // Apply full access policy (dmPolicy/groupPolicy/pairing) — same as handleReactionOnlyInbound. + const bareAccessDecision = resolveAccessDecision(isGroup); + if (bareAccessDecision.decision !== "allow") { + logVerbose( + `signal: bare reaction from unauthorized sender ${senderDisplayBare}, dropping (${bareAccessDecision.reasonCode ?? "policy"})`, + ); + return; + } + // Apply notification-mode gating (off/own/all/allowlist). + const bareReactionTargets = deps.resolveSignalReactionTargets(bareReaction); + const shouldNotifyBare = deps.shouldEmitSignalReactionNotification({ + mode: deps.reactionMode, + account: deps.account, + targets: bareReactionTargets, + sender, + allowlist: deps.reactionAllowlist, + }); + if (!shouldNotifyBare) { + logVerbose(`signal: bare reaction suppressed (reactionMode=${deps.reactionMode})`); + return; + } + const senderName = envelope.sourceName ?? senderDisplayBare; + const senderPeerIdBare = resolveSignalPeerId(sender); + const routeBare = resolveAgentRoute({ + cfg: deps.cfg, + channel: "signal", + accountId: deps.accountId, + peer: { + kind: isGroup ? "group" : "direct", + id: isGroup ? (groupId ?? "unknown") : senderPeerIdBare, + }, + }); + const messageId = typeof targetTimestamp === "number" ? String(targetTimestamp) : "unknown"; + const groupLabel = isGroup ? `${groupName ?? "Signal Group"} id:${groupId}` : undefined; + const text = deps.buildSignalReactionSystemEventText({ + emojiLabel, + actorLabel: senderName, + messageId, + groupLabel, + }); + enqueueSystemEvent(text, { + sessionKey: routeBare.sessionKey, + contextKey: [ + "signal", + "reaction", + "added", + messageId, + senderPeerIdBare, + emojiLabel, + groupId ?? "", + ] + .filter(Boolean) + .join(":"), + }); + } + return; + } + + const hasBodyContent = + Boolean(messageText || quoteText) || Boolean(!reaction && dataMessage?.attachments?.length); + const senderDisplay = formatSignalSenderDisplay(sender); + if ( reaction && handleReactionOnlyInbound({ @@ -737,12 +829,17 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { if (mediaPaths.length > 1) { placeholder = formatAttachmentSummaryPlaceholder(mediaTypes); } else { - const kind = kindFromMime(mediaType ?? undefined); + const kind = mediaType ? kindFromMime(mediaType) : undefined; if (kind) { placeholder = ``; - } else if (attachments.length) { + } else if (mediaPath || attachments.length) { + // A path was resolved (real attachment, unrecognised MIME), fetch + // returned null, or we are intentionally skipping fetching — preserve + // the attachment placeholder so the message is not silently lost. placeholder = ""; } + // No attachments at all (blank reaction envelope): + // Leave placeholder empty; the !bodyText guard below will drop it silently. } const bodyText = messageText || placeholder || dataMessage.quote?.text?.trim() || "";