From a2eceb9b2fd947ad033a6e3a2fa2598ca73a8fdc Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Jul 2026 00:30:29 -0400 Subject: [PATCH] fix(reply): honor channel-declared explicit-reply-tag opt-out (#114268) * fix(reply): honor channel-declared explicit-reply-tag opt-out * test(reply): export getLoadedChannelPlugin from followup-delivery plugins mock * docs(reply): state the named-channel default truthfully and pin it --- .../reply/followup-delivery.test.ts | 1 + src/auto-reply/reply/reply-flow.test.ts | 2 +- src/auto-reply/reply/reply-threading.test.ts | 92 ++++++++++++++++++- src/auto-reply/reply/reply-threading.ts | 12 ++- src/channels/thread-addressing.test.ts | 2 + src/channels/thread-addressing.ts | 16 +++- 6 files changed, 114 insertions(+), 11 deletions(-) diff --git a/src/auto-reply/reply/followup-delivery.test.ts b/src/auto-reply/reply/followup-delivery.test.ts index 43bd5ae5ae65..072370a919b7 100644 --- a/src/auto-reply/reply/followup-delivery.test.ts +++ b/src/auto-reply/reply/followup-delivery.test.ts @@ -6,6 +6,7 @@ import { resolveFollowupDeliveryPayloads } from "./followup-delivery.js"; vi.mock("../../channels/plugins/index.js", () => ({ getChannelPlugin: () => undefined, + getLoadedChannelPlugin: () => undefined, })); const baseConfig = {} as OpenClawConfig; diff --git a/src/auto-reply/reply/reply-flow.test.ts b/src/auto-reply/reply/reply-flow.test.ts index ca7a990d510d..0d9ac52f5589 100644 --- a/src/auto-reply/reply/reply-flow.test.ts +++ b/src/auto-reply/reply/reply-flow.test.ts @@ -577,7 +577,7 @@ describe("createReplyToModeFilterForChannel", () => { expectedReplyToId: undefined, }, { - filter: createReplyToModeFilterForChannel("off", "slack"), + filter: createReplyToModeFilterForChannel("off", "telegram"), input: { text: "hi", replyToId: "1", replyToTag: true }, expectedReplyToId: "1", }, diff --git a/src/auto-reply/reply/reply-threading.test.ts b/src/auto-reply/reply/reply-threading.test.ts index 4a30e2b16667..a47af8eb373c 100644 --- a/src/auto-reply/reply/reply-threading.test.ts +++ b/src/auto-reply/reply/reply-threading.test.ts @@ -2,8 +2,15 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../../config/config.js"; import { setActivePluginRegistry } from "../../plugins/runtime.js"; -import { createTestRegistry } from "../../test-utils/channel-plugins.js"; -import { resolveReplyDeliveryAccountId, resolveReplyToMode } from "./reply-threading.js"; +import { + createChannelTestPluginBase, + createTestRegistry, +} from "../../test-utils/channel-plugins.js"; +import { + createReplyToModeFilterForChannel, + resolveReplyDeliveryAccountId, + resolveReplyToMode, +} from "./reply-threading.js"; const emptyCfg = {} as OpenClawConfig; @@ -128,3 +135,84 @@ describe("resolveReplyToMode", () => { expect(resolveReplyDeliveryAccountId(emptyCfg, "whatsapp", "personal")).toBe("personal"); }); }); + +describe("createReplyToModeFilterForChannel", () => { + beforeEach(() => { + setActivePluginRegistry(createTestRegistry()); + }); + + afterEach(() => { + setActivePluginRegistry(createTestRegistry()); + }); + + it("strips explicit Slack reply tags upstream when replyToMode is off", () => { + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "slack", + source: "test", + plugin: { + ...createChannelTestPluginBase({ id: "slack" }), + threading: { allowExplicitReplyTagsWhenOff: false }, + }, + }, + ]), + ); + + const filter = createReplyToModeFilterForChannel("off", "slack"); + + expect(filter({ text: "hello", replyToId: "message-1", replyToTag: true }).replyToId).toBe( + undefined, + ); + }); + + it("keeps other known-channel defaults and fails closed without a channel", () => { + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "telegram", + source: "test", + plugin: createChannelTestPluginBase({ id: "telegram" }), + }, + ]), + ); + const explicitReply = { text: "hello", replyToId: "message-1", replyToTag: true }; + + expect(createReplyToModeFilterForChannel("off", "telegram")(explicitReply).replyToId).toBe( + "message-1", + ); + expect(createReplyToModeFilterForChannel("off")(explicitReply).replyToId).toBeUndefined(); + }); + + it("allows explicit tags for named channels without a loaded plugin", () => { + // The filter also runs where plugins are not loaded; stripping for unrecognized + // ids would break real channels there. Accepted tradeoff pinned on purpose. + setActivePluginRegistry(createTestRegistry([])); + const explicitReply = { text: "hello", replyToId: "message-1", replyToTag: true }; + + expect( + createReplyToModeFilterForChannel("off", "unloaded-channel")(explicitReply).replyToId, + ).toBe("message-1"); + }); + + it("honors the deprecated allowTagsWhenOff adapter alias", () => { + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "legacy-threading", + source: "test", + plugin: { + ...createChannelTestPluginBase({ id: "legacy-threading" }), + threading: { allowTagsWhenOff: false }, + }, + }, + ]), + ); + + const filter = createReplyToModeFilterForChannel("off", "legacy-threading"); + + expect(filter({ text: "hello", replyToId: "message-1", replyToTag: true }).replyToId).toBe( + undefined, + ); + }); +}); diff --git a/src/auto-reply/reply/reply-threading.ts b/src/auto-reply/reply/reply-threading.ts index 8b053d1dddff..baedbd891bc4 100644 --- a/src/auto-reply/reply/reply-threading.ts +++ b/src/auto-reply/reply/reply-threading.ts @@ -4,6 +4,7 @@ import { normalizeChatType } from "../../channels/chat-type.js"; import { getChannelPlugin } from "../../channels/plugins/index.js"; import type { ChannelThreadingAdapter } from "../../channels/plugins/types.core.js"; import { normalizeAnyChannelId } from "../../channels/registry.js"; +import { getLoadedChannelThreadingAdapter } from "../../channels/thread-addressing.js"; import type { ReplyToMode } from "../../config/types.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { DEFAULT_ACCOUNT_ID } from "../../routing/account-id.js"; @@ -219,10 +220,13 @@ export function createReplyToModeFilterForChannel( channel?: OriginatingChannelType, ) { const normalized = normalizeOptionalLowercaseString(channel); - const isWebchat = normalized === "webchat"; - // Default: allow explicit reply tags/directives even when replyToMode is "off". - // Unknown channels fail closed; internal webchat stays allowed. - const allowExplicitReplyTagsWhenOff = normalized ? true : isWebchat; + const adapter = getLoadedChannelThreadingAdapter(normalized); + // Channels may opt out via their threading adapter. Any named channel defaults to + // allowing explicit tags — including ids with no loaded plugin, because this filter + // also runs where plugins are not loaded and stripping there would break real + // channels. Only an absent channel fails closed. Accepted tradeoff, not an oversight. + const allowExplicitReplyTagsWhenOff = + adapter?.allowExplicitReplyTagsWhenOff ?? adapter?.allowTagsWhenOff ?? Boolean(normalized); return createReplyToModeFilter(mode, { allowExplicitReplyTagsWhenOff, }); diff --git a/src/channels/thread-addressing.test.ts b/src/channels/thread-addressing.test.ts index 5f6004f0cd7d..2ec45273d85b 100644 --- a/src/channels/thread-addressing.test.ts +++ b/src/channels/thread-addressing.test.ts @@ -3,6 +3,7 @@ import { setActivePluginRegistry } from "../plugins/runtime.js"; import { createChannelTestPluginBase, createTestRegistry } from "../test-utils/channel-plugins.js"; import { channelSupportsThreadDelivery, + getLoadedChannelThreadingAdapter, resolveChannelThreadAddressing, } from "./thread-addressing.js"; @@ -35,6 +36,7 @@ describe("resolveChannelThreadAddressing", () => { ); expect(resolveChannelThreadAddressing("messagechat")).toBe("message"); + expect(getLoadedChannelThreadingAdapter("messagechat")?.threadAddressing).toBe("message"); }); }); diff --git a/src/channels/thread-addressing.ts b/src/channels/thread-addressing.ts index 5332aa6b4aff..649c65d0bc06 100644 --- a/src/channels/thread-addressing.ts +++ b/src/channels/thread-addressing.ts @@ -1,11 +1,19 @@ import { getLoadedChannelPlugin } from "./plugins/index.js"; +import type { ChannelThreadingAdapter } from "./plugins/types.core.js"; + +/** Returns the loaded channel's threading adapter without bundled fallback discovery. */ +export function getLoadedChannelThreadingAdapter( + channel?: string | null, +): ChannelThreadingAdapter | undefined { + if (!channel) { + return undefined; + } + return getLoadedChannelPlugin(channel)?.threading; +} /** Resolves where a loaded channel transport keeps thread identity. */ export function resolveChannelThreadAddressing(channel?: string | null): "address" | "message" { - if (!channel) { - return "address"; - } - return getLoadedChannelPlugin(channel)?.threading?.threadAddressing ?? "address"; + return getLoadedChannelThreadingAdapter(channel)?.threadAddressing ?? "address"; } // Thread-addressed delivery must be declared, not inferred: a route can carry a