mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 04:47:03 -06:00
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
This commit is contained in:
committed by
GitHub
parent
fdfe5125a3
commit
a2eceb9b2f
@@ -6,6 +6,7 @@ import { resolveFollowupDeliveryPayloads } from "./followup-delivery.js";
|
||||
|
||||
vi.mock("../../channels/plugins/index.js", () => ({
|
||||
getChannelPlugin: () => undefined,
|
||||
getLoadedChannelPlugin: () => undefined,
|
||||
}));
|
||||
|
||||
const baseConfig = {} as OpenClawConfig;
|
||||
|
||||
@@ -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",
|
||||
},
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user