From 02bd6e4a249d1abfe169db3d1bc8e2486c613b14 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 23 Jan 2026 22:17:14 +0000 Subject: [PATCH] refactor: centralize ack reaction gating --- extensions/bluebubbles/src/monitor.test.ts | 4 + extensions/bluebubbles/src/monitor.ts | 27 ++-- .../matrix/src/matrix/monitor/handler.ts | 27 ++-- src/channels/ack-reactions.test.ts | 134 ++++++++++++++++++ src/channels/ack-reactions.ts | 27 ++++ .../monitor/message-handler.process.ts | 30 ++-- src/plugin-sdk/index.ts | 2 + src/plugins/runtime/index.ts | 4 + src/plugins/runtime/types.ts | 4 + src/slack/monitor/message-handler/prepare.ts | 31 ++-- src/telegram/bot-message-context.ts | 28 ++-- 11 files changed, 253 insertions(+), 65 deletions(-) create mode 100644 src/channels/ack-reactions.test.ts create mode 100644 src/channels/ack-reactions.ts diff --git a/extensions/bluebubbles/src/monitor.test.ts b/extensions/bluebubbles/src/monitor.test.ts index fa40e82a7..1901af351 100644 --- a/extensions/bluebubbles/src/monitor.test.ts +++ b/extensions/bluebubbles/src/monitor.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { IncomingMessage, ServerResponse } from "node:http"; import { EventEmitter } from "node:events"; +import { shouldAckReaction } from "clawdbot/plugin-sdk"; import type { ClawdbotConfig, PluginRuntime } from "clawdbot/plugin-sdk"; import { handleBlueBubblesWebhookRequest, @@ -135,6 +136,9 @@ function createMockRuntime(): PluginRuntime { buildMentionRegexes: mockBuildMentionRegexes as unknown as PluginRuntime["channel"]["mentions"]["buildMentionRegexes"], matchesMentionPatterns: mockMatchesMentionPatterns as unknown as PluginRuntime["channel"]["mentions"]["matchesMentionPatterns"], }, + reactions: { + shouldAckReaction, + }, groups: { resolveGroupPolicy: mockResolveGroupPolicy as unknown as PluginRuntime["channel"]["groups"]["resolveGroupPolicy"], resolveRequireMention: mockResolveRequireMention as unknown as PluginRuntime["channel"]["groups"]["resolveRequireMention"], diff --git a/extensions/bluebubbles/src/monitor.ts b/extensions/bluebubbles/src/monitor.ts index 81a921ca9..325599c97 100644 --- a/extensions/bluebubbles/src/monitor.ts +++ b/extensions/bluebubbles/src/monitor.ts @@ -1521,19 +1521,20 @@ async function processMessage( core, runtime, }); - const shouldAckReaction = () => { - if (!ackReactionValue) return false; - if (ackReactionScope === "all") return true; - if (ackReactionScope === "direct") return !isGroup; - if (ackReactionScope === "group-all") return isGroup; - if (ackReactionScope === "group-mentions") { - if (!isGroup) return false; - if (!requireMention) return false; - if (!canDetectMention) return false; - return effectiveWasMentioned; - } - return false; - }; + const shouldAckReaction = () => + Boolean( + ackReactionValue && + core.channel.reactions.shouldAckReaction({ + scope: ackReactionScope, + isDirect: !isGroup, + isGroup, + isMentionableGroup: isGroup, + requireMention: Boolean(requireMention), + canDetectMention, + effectiveWasMentioned, + shouldBypassMention, + }), + ); const ackMessageId = message.messageId?.trim() || ""; const ackReactionPromise = shouldAckReaction() && ackMessageId && chatGuidForActions && ackReactionValue diff --git a/extensions/matrix/src/matrix/monitor/handler.ts b/extensions/matrix/src/matrix/monitor/handler.ts index 49deabbf8..10db2be20 100644 --- a/extensions/matrix/src/matrix/monitor/handler.ts +++ b/extensions/matrix/src/matrix/monitor/handler.ts @@ -410,6 +410,7 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam !hasExplicitMention && commandAuthorized && core.channel.text.hasControlCommand(bodyText); + const canDetectMention = mentionRegexes.length > 0 || hasExplicitMention; if (isRoom && shouldRequireMention && !wasMentioned && !shouldBypassMention) { logger.info({ roomId, reason: "no-mention" }, "skipping room message"); return; @@ -515,18 +516,20 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam const ackReaction = (cfg.messages?.ackReaction ?? "").trim(); const ackScope = cfg.messages?.ackReactionScope ?? "group-mentions"; - const shouldAckReaction = () => { - if (!ackReaction) return false; - if (ackScope === "all") return true; - if (ackScope === "direct") return isDirectMessage; - if (ackScope === "group-all") return isRoom; - if (ackScope === "group-mentions") { - if (!isRoom) return false; - if (!shouldRequireMention) return false; - return wasMentioned || shouldBypassMention; - } - return false; - }; + const shouldAckReaction = () => + Boolean( + ackReaction && + core.channel.reactions.shouldAckReaction({ + scope: ackScope, + isDirect: isDirectMessage, + isGroup: isRoom, + isMentionableGroup: isRoom, + requireMention: Boolean(shouldRequireMention), + canDetectMention, + effectiveWasMentioned: wasMentioned || shouldBypassMention, + shouldBypassMention, + }), + ); if (shouldAckReaction() && messageId) { reactMatrixMessage(roomId, messageId, ackReaction, client).catch((err) => { logVerboseMessage(`matrix react failed for room ${roomId}: ${String(err)}`); diff --git a/src/channels/ack-reactions.test.ts b/src/channels/ack-reactions.test.ts new file mode 100644 index 000000000..14333e965 --- /dev/null +++ b/src/channels/ack-reactions.test.ts @@ -0,0 +1,134 @@ +import { describe, expect, it } from "vitest"; + +import { shouldAckReaction } from "./ack-reactions.js"; + +describe("shouldAckReaction", () => { + it("honors direct and group-all scopes", () => { + expect( + shouldAckReaction({ + scope: "direct", + isDirect: true, + isGroup: false, + isMentionableGroup: false, + requireMention: false, + canDetectMention: false, + effectiveWasMentioned: false, + }), + ).toBe(true); + + expect( + shouldAckReaction({ + scope: "group-all", + isDirect: false, + isGroup: true, + isMentionableGroup: true, + requireMention: false, + canDetectMention: false, + effectiveWasMentioned: false, + }), + ).toBe(true); + }); + + it("skips when scope is off or none", () => { + expect( + shouldAckReaction({ + scope: "off", + isDirect: true, + isGroup: true, + isMentionableGroup: true, + requireMention: true, + canDetectMention: true, + effectiveWasMentioned: true, + }), + ).toBe(false); + + expect( + shouldAckReaction({ + scope: "none", + isDirect: true, + isGroup: true, + isMentionableGroup: true, + requireMention: true, + canDetectMention: true, + effectiveWasMentioned: true, + }), + ).toBe(false); + }); + + it("defaults to group-mentions gating", () => { + expect( + shouldAckReaction({ + scope: undefined, + isDirect: false, + isGroup: true, + isMentionableGroup: true, + requireMention: true, + canDetectMention: true, + effectiveWasMentioned: true, + }), + ).toBe(true); + }); + + it("requires mention gating for group-mentions", () => { + expect( + shouldAckReaction({ + scope: "group-mentions", + isDirect: false, + isGroup: true, + isMentionableGroup: true, + requireMention: false, + canDetectMention: true, + effectiveWasMentioned: true, + }), + ).toBe(false); + + expect( + shouldAckReaction({ + scope: "group-mentions", + isDirect: false, + isGroup: true, + isMentionableGroup: true, + requireMention: true, + canDetectMention: false, + effectiveWasMentioned: true, + }), + ).toBe(false); + + expect( + shouldAckReaction({ + scope: "group-mentions", + isDirect: false, + isGroup: true, + isMentionableGroup: false, + requireMention: true, + canDetectMention: true, + effectiveWasMentioned: true, + }), + ).toBe(false); + + expect( + shouldAckReaction({ + scope: "group-mentions", + isDirect: false, + isGroup: true, + isMentionableGroup: true, + requireMention: true, + canDetectMention: true, + effectiveWasMentioned: true, + }), + ).toBe(true); + + expect( + shouldAckReaction({ + scope: "group-mentions", + isDirect: false, + isGroup: true, + isMentionableGroup: true, + requireMention: true, + canDetectMention: true, + effectiveWasMentioned: false, + shouldBypassMention: true, + }), + ).toBe(true); + }); +}); diff --git a/src/channels/ack-reactions.ts b/src/channels/ack-reactions.ts new file mode 100644 index 000000000..dfe2f1879 --- /dev/null +++ b/src/channels/ack-reactions.ts @@ -0,0 +1,27 @@ +export type AckReactionScope = "all" | "direct" | "group-all" | "group-mentions" | "off" | "none"; + +export type AckReactionGateParams = { + scope: AckReactionScope | undefined; + isDirect: boolean; + isGroup: boolean; + isMentionableGroup: boolean; + requireMention: boolean; + canDetectMention: boolean; + effectiveWasMentioned: boolean; + shouldBypassMention?: boolean; +}; + +export function shouldAckReaction(params: AckReactionGateParams): boolean { + const scope = params.scope ?? "group-mentions"; + if (scope === "off" || scope === "none") return false; + if (scope === "all") return true; + if (scope === "direct") return params.isDirect; + if (scope === "group-all") return params.isGroup; + if (scope === "group-mentions") { + if (!params.isMentionableGroup) return false; + if (!params.requireMention) return false; + if (!params.canDetectMention) return false; + return params.effectiveWasMentioned || params.shouldBypassMention === true; + } + return false; +} diff --git a/src/discord/monitor/message-handler.process.ts b/src/discord/monitor/message-handler.process.ts index ad1e4baea..3a6b650cd 100644 --- a/src/discord/monitor/message-handler.process.ts +++ b/src/discord/monitor/message-handler.process.ts @@ -8,6 +8,7 @@ import { extractShortModelName, type ResponsePrefixContext, } from "../../auto-reply/reply/response-prefix-template.js"; +import { shouldAckReaction as shouldAckReactionGate } from "../../channels/ack-reactions.js"; import { formatInboundEnvelope, formatThreadStarterEnvelope, @@ -73,6 +74,7 @@ export async function processDiscordMessage(ctx: DiscordMessagePreflightContext) shouldRequireMention, canDetectMention, effectiveWasMentioned, + shouldBypassMention, threadChannel, threadParentId, threadParentName, @@ -95,20 +97,20 @@ export async function processDiscordMessage(ctx: DiscordMessagePreflightContext) } const ackReaction = resolveAckReaction(cfg, route.agentId); const removeAckAfterReply = cfg.messages?.removeAckAfterReply ?? false; - const shouldAckReaction = () => { - if (!ackReaction) return false; - if (ackReactionScope === "all") return true; - if (ackReactionScope === "direct") return isDirectMessage; - const isGroupChat = isGuildMessage || isGroupDm; - if (ackReactionScope === "group-all") return isGroupChat; - if (ackReactionScope === "group-mentions") { - if (!isGuildMessage) return false; - if (!shouldRequireMention) return false; - if (!canDetectMention) return false; - return effectiveWasMentioned; - } - return false; - }; + const shouldAckReaction = () => + Boolean( + ackReaction && + shouldAckReactionGate({ + scope: ackReactionScope, + isDirect: isDirectMessage, + isGroup: isGuildMessage || isGroupDm, + isMentionableGroup: isGuildMessage, + requireMention: Boolean(shouldRequireMention), + canDetectMention, + effectiveWasMentioned, + shouldBypassMention, + }), + ); const ackReactionPromise = shouldAckReaction() ? reactMessageDiscord(message.channelId, message.id, ackReaction, { rest: client.rest, diff --git a/src/plugin-sdk/index.ts b/src/plugin-sdk/index.ts index 72bb72422..7de6df564 100644 --- a/src/plugin-sdk/index.ts +++ b/src/plugin-sdk/index.ts @@ -117,6 +117,8 @@ export { resolveMentionGating, resolveMentionGatingWithBypass, } from "../channels/mention-gating.js"; +export type { AckReactionGateParams, AckReactionScope } from "../channels/ack-reactions.js"; +export { shouldAckReaction } from "../channels/ack-reactions.js"; export { resolveChannelMediaMaxBytes } from "../channels/plugins/media-limits.js"; export type { NormalizedLocation } from "../channels/location.js"; export { formatLocationText, toLocationContext } from "../channels/location.js"; diff --git a/src/plugins/runtime/index.ts b/src/plugins/runtime/index.ts index 6bb10984b..e0ade62ca 100644 --- a/src/plugins/runtime/index.ts +++ b/src/plugins/runtime/index.ts @@ -25,6 +25,7 @@ import { resolveEffectiveMessagesConfig, resolveHumanDelayConfig } from "../../a import { createMemoryGetTool, createMemorySearchTool } from "../../agents/tools/memory-tool.js"; import { handleSlackAction } from "../../agents/tools/slack-actions.js"; import { handleWhatsAppAction } from "../../agents/tools/whatsapp-actions.js"; +import { shouldAckReaction } from "../../channels/ack-reactions.js"; import { resolveCommandAuthorizedFromAuthorizers } from "../../channels/command-gating.js"; import { discordMessageActions } from "../../channels/plugins/actions/discord.js"; import { telegramMessageActions } from "../../channels/plugins/actions/telegram.js"; @@ -198,6 +199,9 @@ export function createPluginRuntime(): PluginRuntime { buildMentionRegexes, matchesMentionPatterns, }, + reactions: { + shouldAckReaction, + }, groups: { resolveGroupPolicy: resolveChannelGroupPolicy, resolveRequireMention: resolveChannelGroupRequireMention, diff --git a/src/plugins/runtime/types.ts b/src/plugins/runtime/types.ts index 1f321d04b..86c7caa55 100644 --- a/src/plugins/runtime/types.ts +++ b/src/plugins/runtime/types.ts @@ -19,6 +19,7 @@ type SaveMediaBuffer = typeof import("../../media/store.js").saveMediaBuffer; type BuildMentionRegexes = typeof import("../../auto-reply/reply/mentions.js").buildMentionRegexes; type MatchesMentionPatterns = typeof import("../../auto-reply/reply/mentions.js").matchesMentionPatterns; +type ShouldAckReaction = typeof import("../../channels/ack-reactions.js").shouldAckReaction; type ResolveChannelGroupPolicy = typeof import("../../config/group-policy.js").resolveChannelGroupPolicy; type ResolveChannelGroupRequireMention = @@ -211,6 +212,9 @@ export type PluginRuntime = { buildMentionRegexes: BuildMentionRegexes; matchesMentionPatterns: MatchesMentionPatterns; }; + reactions: { + shouldAckReaction: ShouldAckReaction; + }; groups: { resolveGroupPolicy: ResolveChannelGroupPolicy; resolveRequireMention: ResolveChannelGroupRequireMention; diff --git a/src/slack/monitor/message-handler/prepare.ts b/src/slack/monitor/message-handler/prepare.ts index 767ecea89..7bea8a170 100644 --- a/src/slack/monitor/message-handler/prepare.ts +++ b/src/slack/monitor/message-handler/prepare.ts @@ -19,6 +19,10 @@ import { buildPairingReply } from "../../../pairing/pairing-messages.js"; import { upsertChannelPairingRequest } from "../../../pairing/pairing-store.js"; import { resolveAgentRoute } from "../../../routing/resolve-route.js"; import { resolveThreadSessionKeys } from "../../../routing/session-key.js"; +import { + shouldAckReaction as shouldAckReactionGate, + type AckReactionScope, +} from "../../../channels/ack-reactions.js"; import { resolveMentionGatingWithBypass } from "../../../channels/mention-gating.js"; import { resolveConversationLabel } from "../../../channels/conversation-label.js"; import { resolveControlCommandGate } from "../../../channels/command-gating.js"; @@ -324,19 +328,20 @@ export async function prepareSlackMessage(params: { const ackReaction = resolveAckReaction(cfg, route.agentId); const ackReactionValue = ackReaction ?? ""; - const shouldAckReaction = () => { - if (!ackReaction) return false; - if (ctx.ackReactionScope === "all") return true; - if (ctx.ackReactionScope === "direct") return isDirectMessage; - if (ctx.ackReactionScope === "group-all") return isRoomish; - if (ctx.ackReactionScope === "group-mentions") { - if (!isRoom) return false; - if (!shouldRequireMention) return false; - if (!canDetectMention) return false; - return effectiveWasMentioned; - } - return false; - }; + const shouldAckReaction = () => + Boolean( + ackReaction && + shouldAckReactionGate({ + scope: ctx.ackReactionScope as AckReactionScope | undefined, + isDirect: isDirectMessage, + isGroup: isRoomish, + isMentionableGroup: isRoom, + requireMention: Boolean(shouldRequireMention), + canDetectMention, + effectiveWasMentioned, + shouldBypassMention: mentionGate.shouldBypassMention, + }), + ); const ackReactionMessageTs = message.ts; const ackReactionPromise = diff --git a/src/telegram/bot-message-context.ts b/src/telegram/bot-message-context.ts index dc8039a18..5b67e7eb1 100644 --- a/src/telegram/bot-message-context.ts +++ b/src/telegram/bot-message-context.ts @@ -24,6 +24,7 @@ import type { DmPolicy, TelegramGroupConfig, TelegramTopicConfig } from "../conf import { logVerbose, shouldLogVerbose } from "../globals.js"; import { recordChannelActivity } from "../infra/channel-activity.js"; import { resolveAgentRoute } from "../routing/resolve-route.js"; +import { shouldAckReaction as shouldAckReactionGate } from "../channels/ack-reactions.js"; import { resolveMentionGatingWithBypass } from "../channels/mention-gating.js"; import { resolveControlCommandGate } from "../channels/command-gating.js"; import { @@ -369,19 +370,20 @@ export const buildTelegramMessageContext = async ({ // ACK reactions const ackReaction = resolveAckReaction(cfg, route.agentId); const removeAckAfterReply = cfg.messages?.removeAckAfterReply ?? false; - const shouldAckReaction = () => { - if (!ackReaction) return false; - if (ackReactionScope === "all") return true; - if (ackReactionScope === "direct") return !isGroup; - if (ackReactionScope === "group-all") return isGroup; - if (ackReactionScope === "group-mentions") { - if (!isGroup) return false; - if (!requireMention) return false; - if (!canDetectMention) return false; - return effectiveWasMentioned; - } - return false; - }; + const shouldAckReaction = () => + Boolean( + ackReaction && + shouldAckReactionGate({ + scope: ackReactionScope, + isDirect: !isGroup, + isGroup, + isMentionableGroup: isGroup, + requireMention: Boolean(requireMention), + canDetectMention, + effectiveWasMentioned, + shouldBypassMention: mentionGate.shouldBypassMention, + }), + ); const api = bot.api as unknown as { setMessageReaction?: ( chatId: number | string,