Merge 613a116d97a8c5ebe2d360138a9e7f2f93c30e03 into d78e13f545136fcbba1feceecc5e0485a06c33a6
This commit is contained in:
commit
2d2ab86ab3
@ -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("../../../../src/auto-reply/dispatch.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../../../../src/auto-reply/dispatch.js")>();
|
||||
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<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);
|
||||
});
|
||||
});
|
||||
@ -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 <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,
|
||||
@ -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 = `<media:${kind}>`;
|
||||
} 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 = "<media:attachment>";
|
||||
}
|
||||
// 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() || "";
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user