Ayuriel/openclaw
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(signal): silence <media:unknown> dispatch from blank reaction envelopes

Browse Source 
Root cause: mediaKindFromMime(undefined) returns "unknown" which is
truthy, so the placeholder block was unconditionally setting
placeholder = "<media:unknown>" on every message where no attachment
was fetched. This caused blank-body dataMessages (e.g. Signal emoji
reactions delivered by signal-cli without a reaction sub-field) to be
dispatched to the agent with <media:unknown> as the body text.

Fix: only invoke mediaKindFromMime when mediaType is actually defined.
When kind resolves to "unknown" or no attachment is present, fall back
to "<media:attachment>" (or empty when no attachment at all).

Also adds hasBareReactionField guard: if dataMessage.reaction is
present but isSignalReactionMessage() returned null, surface it as a
system event rather than leaking through. Reaction-removals silently
dropped. Mixed-content messages pass through normally.

UAT confirmed: 👍 long-press reaction on a DM is now silently dropped
with no <media:unknown> response dispatched to the agent.

7 tests: well-formed reactions, bare reactions emitting system events,
timestamp inclusion, reaction-removals, mixed content, plain messages,
reaction with null-contentType attachment.
This commit is contained in:
Dale 2026-03-02 13:16:17 -05:00 committed by root
parent 8a11d0ea32
commit 292a8ef9c4
2 changed files with 344 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

247
extensions/signal/src/monitor/event-handler.emoji-reactions.test.ts Normal file
View File

@ -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 <media:unknown>.
*
* 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("../../auto-reply/dispatch.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("../../auto-reply/dispatch.js")>();
return {
...actual,
dispatchInboundMessage: dispatchInboundMessageMock,
dispatchInboundMessageWithDispatcher: dispatchInboundMessageMock,
dispatchInboundMessageWithBufferedDispatcher: dispatchInboundMessageMock,
};
});
vi.mock("../../infra/system-events.js", () => ({
enqueueSystemEvent: enqueueSystemEventMock,
}));
vi.mock("../../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<typeof r> =>
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);
});
});

103
extensions/signal/src/monitor/event-handler.ts
View File

@ -512,6 +512,7 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
}
const dataMessage = envelope.dataMessage ?? envelope.editMessage?.dataMessage;
const reaction = deps.isSignalReactionMessage(envelope.reactionMessage)
? envelope.reactionMessage
: deps.isSignalReactionMessage(dataMessage?.reaction)
@ -525,9 +526,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 <media:unknown>.
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
// <media:unknown>.
// Resolve full access state early — shared by bare reaction path and normal dispatch.
const { resolveAccessDecision, dmAccess, effectiveDmAllow, effectiveGroupAllow } =
await resolveSignalAccessState({
accountId: deps.accountId,
@ -538,6 +547,84 @@ 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: prefer group info from the reaction payload itself; fall back to dataMessage.groupInfo.
const bareReactionGroupInfo =
(bareReaction as { groupInfo?: { groupId?: string; groupName?: string } | null })
.groupInfo ?? dataMessage?.groupInfo;
const groupId = bareReactionGroupInfo?.groupId ?? undefined;
const groupName = bareReactionGroupInfo?.groupName ?? undefined;
const isGroup = Boolean(groupId);
// Apply full access policy (dmPolicy/groupPolicy) — 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({
@ -774,10 +861,14 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
if (mediaPaths.length > 1) {
placeholder = formatAttachmentSummaryPlaceholder(mediaTypes);
} else {
const kind = kindFromMime(mediaType ?? undefined);
if (kind) {
// Only set placeholder when we actually resolved a mediaType.
// kindFromMime(undefined) returns "unknown" which is truthy — guard against
// that case so null-body messages (e.g. blank reaction envelopes) aren't dispatched
// with <media:unknown> as the body.
const kind = mediaType ? kindFromMime(mediaType) : undefined;
if (kind && kind !== "unknown") {
placeholder = `<media:${kind}>`;
} else if (attachments.length) {
} else if (kind === "unknown" || (!kind && dataMessage.attachments?.length)) {
placeholder = "<media:attachment>";
}
}