From b0ccaa6eecbadeb326bc12e2f42fec5d7540fb24 Mon Sep 17 00:00:00 2001 From: Josh Avant <830519+joshavant@users.noreply.github.com> Date: Wed, 22 Jul 2026 04:30:49 -0500 Subject: [PATCH] fix(signal): bind reactions to normalized target (#112607) --- extensions/signal/src/message-actions.test.ts | 101 +++++++++++++++++- extensions/signal/src/message-actions.ts | 27 +++-- 2 files changed, 114 insertions(+), 14 deletions(-) diff --git a/extensions/signal/src/message-actions.test.ts b/extensions/signal/src/message-actions.test.ts index 3060734b466e..c931d2299a00 100644 --- a/extensions/signal/src/message-actions.test.ts +++ b/extensions/signal/src/message-actions.test.ts @@ -154,14 +154,14 @@ describe("signalMessageActions", () => { name: "normalizes uuid recipients", cfg: { channels: { signal: { account: "+15550001111" } } } as OpenClawConfig, params: { - recipient: "uuid:123e4567-e89b-12d3-a456-426614174000", + to: "uuid:123e4567-e89b-12d3-a456-426614174000", messageId: "123", emoji: "🔥", }, expectedRecipient: "123e4567-e89b-12d3-a456-426614174000", expectedTimestamp: 123, expectedEmoji: "🔥", - expectedOptions: {}, + expectedOptions: { accountId: "default" }, }, { name: "passes groupId and targetAuthor for group reactions", @@ -176,6 +176,7 @@ describe("signalMessageActions", () => { expectedTimestamp: 123, expectedEmoji: "✅", expectedOptions: { + accountId: "default", groupId: "group-id", targetAuthor: "uuid:123e4567-e89b-12d3-a456-426614174000", }, @@ -187,7 +188,7 @@ describe("signalMessageActions", () => { expectedRecipient: "+15559999999", expectedTimestamp: 1737630212345, expectedEmoji: "🔥", - expectedOptions: {}, + expectedOptions: { accountId: "default" }, toolContext: { currentMessageId: "1737630212345" }, }, ] as const; @@ -224,6 +225,86 @@ describe("signalMessageActions", () => { } }); + it("binds provider reactions to the canonical target", async () => { + const cfg = { + channels: { signal: { account: "+15550001111" } }, + } as OpenClawConfig; + + await signalMessageActions.handleAction?.({ + channel: "signal", + action: "react", + params: { + to: "+15559999999", + recipient: "+15558888888", + messageId: "123", + emoji: "✅", + }, + cfg, + }); + + expect(sendReactionSignalMock).toHaveBeenCalledWith( + "+15559999999", + 123, + "✅", + expect.objectContaining({ accountId: "default" }), + ); + + await signalMessageActions.handleAction?.({ + channel: "signal", + action: "react", + params: { + to: "+15559999999", + recipient: "+15558888888", + messageId: "123", + emoji: "✅", + remove: true, + }, + cfg, + }); + + expect(removeReactionSignalMock).toHaveBeenCalledWith( + "+15559999999", + 123, + "✅", + expect.objectContaining({ accountId: "default" }), + ); + }); + + it.each([ + { + name: "disabled", + cfg: { + channels: { + signal: { + account: "+15550001111", + accounts: { work: { enabled: false, account: "+15550002222" } }, + }, + }, + }, + accountId: "work", + error: /account "work" is disabled/, + }, + { + name: "unconfigured", + cfg: { channels: { signal: {} } }, + accountId: "default", + error: /account "default" is not configured/, + }, + ])("rejects $name accounts before provider dispatch", async ({ cfg, accountId, error }) => { + await expect( + signalMessageActions.handleAction?.({ + channel: "signal", + action: "react", + params: { to: "+15559999999", messageId: "123", emoji: "✅" }, + cfg: cfg as OpenClawConfig, + accountId, + }), + ).rejects.toThrow(error); + + expect(sendReactionSignalMock).not.toHaveBeenCalled(); + expect(removeReactionSignalMock).not.toHaveBeenCalled(); + }); + it("rejects invalid reaction inputs before dispatch", async () => { const cfg = { channels: { signal: { account: "+15550001111" } }, @@ -256,5 +337,19 @@ describe("signalMessageActions", () => { cfg, }), ).rejects.toThrow(/targetAuthor/); + + await expect( + signalMessageActions.handleAction?.({ + channel: "signal", + action: "react", + params: { + recipient: "+15559999999", + messageId: "123", + emoji: "✅", + }, + cfg, + }), + ).rejects.toThrow(/recipient.*required/); + expect(sendReactionSignalMock).not.toHaveBeenCalled(); }); }); diff --git a/extensions/signal/src/message-actions.ts b/extensions/signal/src/message-actions.ts index e295e50769ca..b0285b649fc2 100644 --- a/extensions/signal/src/message-actions.ts +++ b/extensions/signal/src/message-actions.ts @@ -116,9 +116,17 @@ export const signalMessageActions: ChannelMessageActionAdapter = { } if (action === "react") { + const account = resolveSignalAccount({ cfg, accountId }); + if (!account.enabled) { + throw new Error(`Signal account "${account.accountId}" is disabled.`); + } + if (!account.configured) { + throw new Error(`Signal account "${account.accountId}" is not configured.`); + } + const reactionLevelInfo = resolveSignalReactionLevel({ cfg, - accountId: accountId ?? undefined, + accountId: account.accountId, }); if (!reactionLevelInfo.agentReactionsEnabled) { throw new Error( @@ -127,18 +135,15 @@ export const signalMessageActions: ChannelMessageActionAdapter = { ); } - const actionConfig = resolveSignalAccount({ cfg, accountId }).config.actions; - const isActionEnabled = createActionGate(actionConfig); + const isActionEnabled = createActionGate(account.config.actions); if (!isActionEnabled("reactions")) { throw new Error("Signal reactions are disabled via actions.reactions."); } - const recipientRaw = - readStringParam(params, "recipient") ?? - readStringParam(params, "to", { - required: true, - label: "recipient (UUID, phone number, or group)", - }); + const recipientRaw = readStringParam(params, "to", { + required: true, + label: "recipient (UUID, phone number, or group)", + }); const target = resolveSignalReactionTarget(recipientRaw); if (!target.recipient && !target.groupId) { throw new Error("recipient or group required"); @@ -171,7 +176,7 @@ export const signalMessageActions: ChannelMessageActionAdapter = { } return await mutateSignalReaction({ cfg, - accountId: accountId ?? undefined, + accountId: account.accountId, target, timestamp, emoji, @@ -186,7 +191,7 @@ export const signalMessageActions: ChannelMessageActionAdapter = { } return await mutateSignalReaction({ cfg, - accountId: accountId ?? undefined, + accountId: account.accountId, target, timestamp, emoji,