From b5e8460912af971934a25446f4723229496cf1da Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 25 Aug 2026 01:06:49 -0700 Subject: [PATCH] refactor(signal): unify reaction recipient and mutation paths (#129110) --- extensions/signal/src/message-actions.test.ts | 30 +++++++++- extensions/signal/src/message-actions.ts | 59 ++++--------------- extensions/signal/src/normalize.ts | 8 +++ extensions/signal/src/send-reactions.test.ts | 34 +++++++---- extensions/signal/src/send-reactions.ts | 54 +++-------------- 5 files changed, 77 insertions(+), 108 deletions(-) diff --git a/extensions/signal/src/message-actions.test.ts b/extensions/signal/src/message-actions.test.ts index c931d2299a00..9ce8f0b8b654 100644 --- a/extensions/signal/src/message-actions.test.ts +++ b/extensions/signal/src/message-actions.test.ts @@ -163,6 +163,19 @@ describe("signalMessageActions", () => { expectedEmoji: "🔥", expectedOptions: { accountId: "default" }, }, + { + name: "preserves UUID case while stripping mixed-case Signal and UUID prefixes", + cfg: { channels: { signal: { account: "+15550001111" } } } as OpenClawConfig, + params: { + to: " SiGnAl: UuId:123E4567-E89B-12D3-A456-426614174000 ", + messageId: "123", + emoji: "🔥", + }, + expectedRecipient: "123E4567-E89B-12D3-A456-426614174000", + expectedTimestamp: 123, + expectedEmoji: "🔥", + expectedOptions: { accountId: "default" }, + }, { name: "passes groupId and targetAuthor for group reactions", cfg: { channels: { signal: { account: "+15550001111" } } } as OpenClawConfig, @@ -230,7 +243,7 @@ describe("signalMessageActions", () => { channels: { signal: { account: "+15550001111" } }, } as OpenClawConfig; - await signalMessageActions.handleAction?.({ + const added = await signalMessageActions.handleAction?.({ channel: "signal", action: "react", params: { @@ -248,8 +261,9 @@ describe("signalMessageActions", () => { "✅", expect.objectContaining({ accountId: "default" }), ); + expect(added?.details).toEqual({ ok: true, added: "✅" }); - await signalMessageActions.handleAction?.({ + const removed = await signalMessageActions.handleAction?.({ channel: "signal", action: "react", params: { @@ -268,6 +282,7 @@ describe("signalMessageActions", () => { "✅", expect.objectContaining({ accountId: "default" }), ); + expect(removed?.details).toEqual({ ok: true, removed: "✅" }); }); it.each([ @@ -329,6 +344,17 @@ describe("signalMessageActions", () => { ).rejects.toThrow(/Invalid messageId/); expect(sendReactionSignalMock).not.toHaveBeenCalled(); + for (const remove of [false, true]) { + await expect( + signalMessageActions.handleAction?.({ + channel: "signal", + action: "react", + params: { to: "+15559999999", messageId: "123", remove }, + cfg, + }), + ).rejects.toThrow(`Emoji required to ${remove ? "remove" : "add"} reaction.`); + } + await expect( signalMessageActions.handleAction?.({ channel: "signal", diff --git a/extensions/signal/src/message-actions.ts b/extensions/signal/src/message-actions.ts index 5dabe4ae0d7d..5a70703af3f9 100644 --- a/extensions/signal/src/message-actions.ts +++ b/extensions/signal/src/message-actions.ts @@ -13,32 +13,17 @@ import { parseStrictNonNegativeInteger } from "openclaw/plugin-sdk/number-runtim import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; import { removeReactionSignal, sendReactionSignal } from "../reaction-runtime-api.js"; import { listEnabledSignalAccounts, resolveSignalAccount } from "./accounts.js"; +import { normalizeSignalReactionRecipient } from "./normalize.js"; import { resolveSignalReactionLevel } from "./reaction-level.js"; const providerId = "signal"; const GROUP_PREFIX = "group:"; -function normalizeSignalReactionRecipient(raw: string): string { - const trimmed = raw.trim(); - if (!trimmed) { - return trimmed; - } - const withoutSignal = trimmed.replace(/^signal:/i, "").trim(); - if (!withoutSignal) { - return withoutSignal; - } - if (normalizeLowercaseStringOrEmpty(withoutSignal).startsWith("uuid:")) { - return withoutSignal.slice("uuid:".length).trim(); - } - return withoutSignal; -} - function resolveSignalReactionTarget(raw: string): { recipient?: string; groupId?: string } { - const trimmed = raw.trim(); - if (!trimmed) { - return {}; - } - const withoutSignal = trimmed.replace(/^signal:/i, "").trim(); + const withoutSignal = raw + .trim() + .replace(/^signal:/i, "") + .trim(); if (!withoutSignal) { return {}; } @@ -66,17 +51,9 @@ async function mutateSignalReaction(params: { targetAuthor: params.targetAuthor, targetAuthorUuid: params.targetAuthorUuid, }; - if (params.remove) { - await removeReactionSignal( - params.target.recipient ?? "", - params.timestamp, - params.emoji, - options, - ); - return jsonResult({ ok: true, removed: params.emoji }); - } - await sendReactionSignal(params.target.recipient ?? "", params.timestamp, params.emoji, options); - return jsonResult({ ok: true, added: params.emoji }); + const mutateReaction = params.remove ? removeReactionSignal : sendReactionSignal; + await mutateReaction(params.target.recipient ?? "", params.timestamp, params.emoji, options); + return jsonResult({ ok: true, [params.remove ? "removed" : "added"]: params.emoji }); } export const signalMessageActions: ChannelMessageActionAdapter = { @@ -174,24 +151,8 @@ export const signalMessageActions: ChannelMessageActionAdapter = { throw new Error(`Invalid messageId: ${messageId}. Expected numeric timestamp.`); } - if (remove) { - if (!emoji) { - throw new Error("Emoji required to remove reaction."); - } - return await mutateSignalReaction({ - cfg, - accountId: account.accountId, - target, - timestamp, - emoji, - remove: true, - targetAuthor, - targetAuthorUuid, - }); - } - if (!emoji) { - throw new Error("Emoji required to add reaction."); + throw new Error(`Emoji required to ${remove ? "remove" : "add"} reaction.`); } return await mutateSignalReaction({ cfg, @@ -199,7 +160,7 @@ export const signalMessageActions: ChannelMessageActionAdapter = { target, timestamp, emoji, - remove: false, + remove: Boolean(remove), targetAuthor, targetAuthorUuid, }); diff --git a/extensions/signal/src/normalize.ts b/extensions/signal/src/normalize.ts index 023e87a0c7f4..5aa4c983d6c4 100644 --- a/extensions/signal/src/normalize.ts +++ b/extensions/signal/src/normalize.ts @@ -4,6 +4,14 @@ import { normalizeStringEntries, } from "openclaw/plugin-sdk/string-coerce-runtime"; +export function normalizeSignalReactionRecipient(raw: string): string { + const withoutSignal = raw + .trim() + .replace(/^signal:/i, "") + .trim(); + return /^uuid:/i.test(withoutSignal) ? withoutSignal.slice("uuid:".length).trim() : withoutSignal; +} + export function normalizeSignalMessagingTarget(raw: string): string | undefined { const trimmed = raw.trim(); if (!trimmed) { diff --git a/extensions/signal/src/send-reactions.test.ts b/extensions/signal/src/send-reactions.test.ts index d723199745a5..0cc72477b4e0 100644 --- a/extensions/signal/src/send-reactions.test.ts +++ b/extensions/signal/src/send-reactions.test.ts @@ -69,18 +69,32 @@ describe("sendReactionSignal", () => { rpcMock.mockClear().mockResolvedValue({ timestamp: 123 }); }); - it("uses recipients array and targetAuthor for uuid dms", async () => { - await sendReactionSignal("uuid:123e4567-e89b-12d3-a456-426614174000", 123, "🔥", { - cfg: SIGNAL_TEST_CFG, - }); + it.each([ + { + name: "UUID", + recipient: "uuid:123e4567-e89b-12d3-a456-426614174000", + expectedRecipient: "123e4567-e89b-12d3-a456-426614174000", + }, + { + name: "mixed-case Signal and UUID prefixes", + recipient: " SiGnAl: UuId:123E4567-E89B-12D3-A456-426614174000 ", + expectedRecipient: "123E4567-E89B-12D3-A456-426614174000", + }, + { + name: "Signal-prefixed phone number", + recipient: " SiGnAl: +15551230000 ", + expectedRecipient: "+15551230000", + }, + ])("uses recipients and targetAuthor for $name DMs", async ({ recipient, expectedRecipient }) => { + await sendReactionSignal(recipient, 123, "🔥", { cfg: SIGNAL_TEST_CFG }); expect(rpcMock).toHaveBeenCalledWith( "sendReaction", { emoji: "🔥", targetTimestamp: 123, - targetAuthor: "123e4567-e89b-12d3-a456-426614174000", - recipients: ["123e4567-e89b-12d3-a456-426614174000"], + targetAuthor: expectedRecipient, + recipients: [expectedRecipient], account: "+15550001111", }, { @@ -90,9 +104,9 @@ describe("sendReactionSignal", () => { }, ); const params = requireRpcParams(); - expect(params.recipients).toEqual(["123e4567-e89b-12d3-a456-426614174000"]); + expect(params.recipients).toEqual([expectedRecipient]); expect(params.groupIds).toBeUndefined(); - expect(params.targetAuthor).toBe("123e4567-e89b-12d3-a456-426614174000"); + expect(params.targetAuthor).toBe(expectedRecipient); expect(params).not.toHaveProperty("recipient"); expect(params).not.toHaveProperty("groupId"); }); @@ -101,13 +115,13 @@ describe("sendReactionSignal", () => { await sendReactionSignal("", 123, "✅", { cfg: SIGNAL_TEST_CFG, groupId: "group-id", - targetAuthorUuid: "uuid:123e4567-e89b-12d3-a456-426614174000", + targetAuthorUuid: " SiGnAl: UuId:123E4567-E89B-12D3-A456-426614174000 ", }); const params = requireRpcParams(); expect(params.recipients).toBeUndefined(); expect(params.groupIds).toEqual(["group-id"]); - expect(params.targetAuthor).toBe("123e4567-e89b-12d3-a456-426614174000"); + expect(params.targetAuthor).toBe("123E4567-E89B-12D3-A456-426614174000"); }); it("honors an explicit container endpoint override", async () => { diff --git a/extensions/signal/src/send-reactions.ts b/extensions/signal/src/send-reactions.ts index 3e1fc7ecac50..dd4e4a3821f8 100644 --- a/extensions/signal/src/send-reactions.ts +++ b/extensions/signal/src/send-reactions.ts @@ -4,9 +4,9 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { requireRuntimeConfig } from "openclaw/plugin-sdk/plugin-config-runtime"; -import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; import { resolveSignalAccount } from "./accounts.js"; import { signalRpcRequest, type SignalTransportKind } from "./client-adapter.js"; +import { normalizeSignalReactionRecipient } from "./normalize.js"; import { resolveSignalRpcContext } from "./rpc-context.js"; export type SignalReactionOpts = { @@ -26,44 +26,6 @@ export type SignalReactionResult = { timestamp?: number; }; -function normalizeSignalId(raw: string): string { - const trimmed = raw.trim(); - if (!trimmed) { - return ""; - } - return trimmed.replace(/^signal:/i, "").trim(); -} - -function normalizeSignalUuid(raw: string): string { - const trimmed = normalizeSignalId(raw); - if (!trimmed) { - return ""; - } - if (normalizeLowercaseStringOrEmpty(trimmed).startsWith("uuid:")) { - return trimmed.slice("uuid:".length).trim(); - } - return trimmed; -} - -function resolveTargetAuthorParams(params: { - targetAuthor?: string; - targetAuthorUuid?: string; - fallback?: string; -}): { targetAuthor?: string } { - const candidates = [params.targetAuthor, params.targetAuthorUuid, params.fallback]; - for (const candidate of candidates) { - const raw = candidate?.trim(); - if (!raw) { - continue; - } - const normalized = normalizeSignalUuid(raw); - if (normalized) { - return { targetAuthor: normalized }; - } - } - return {}; -} - async function sendReactionSignalCore(params: { recipient: string; targetTimestamp: number; @@ -78,7 +40,7 @@ async function sendReactionSignalCore(params: { }); const { baseUrl, account } = resolveSignalRpcContext(params.opts, accountInfo); - const normalizedRecipient = normalizeSignalUuid(params.recipient); + const normalizedRecipient = normalizeSignalReactionRecipient(params.recipient); const groupId = params.opts.groupId?.trim(); const operation = `Signal reaction${params.remove ? " removal" : ""}`; if (!normalizedRecipient && !groupId) { @@ -92,12 +54,10 @@ async function sendReactionSignalCore(params: { throw new Error(`Emoji is required for ${operation}`); } - const targetAuthorParams = resolveTargetAuthorParams({ - targetAuthor: params.opts.targetAuthor, - targetAuthorUuid: params.opts.targetAuthorUuid, - fallback: normalizedRecipient, - }); - if (groupId && !targetAuthorParams.targetAuthor) { + const targetAuthor = [params.opts.targetAuthor, params.opts.targetAuthorUuid, normalizedRecipient] + .map((candidate) => normalizeSignalReactionRecipient(candidate ?? "")) + .find(Boolean); + if (groupId && !targetAuthor) { throw new Error( `targetAuthor is required for group reaction${params.remove ? " removal" : "s"}`, ); @@ -107,7 +67,7 @@ async function sendReactionSignalCore(params: { emoji: normalizedEmoji, targetTimestamp: params.targetTimestamp, ...(params.remove ? { remove: true } : {}), - ...targetAuthorParams, + ...(targetAuthor ? { targetAuthor } : {}), }; if (normalizedRecipient) { requestParams.recipients = [normalizedRecipient];