fix(messages): keep group replies tool-only by default

Rewrites the always-on reply handling so group/channel rooms default to message-tool-visible output, while `messages.groupChat.visibleReplies: \"automatic\"` preserves legacy auto-posting.\n\nThanks @scoootscooob.
This commit is contained in:
scoootscooob
2026-04-27 23:36:43 -07:00
committed by GitHub
parent e388f289bf
commit 3c636208b0
46 changed files with 684 additions and 63 deletions

View File

@@ -99,6 +99,52 @@ describe("handleDiscordMessageAction", () => {
);
});
it("falls back to Discord toolContext.currentChannelId for sends", async () => {
await handleDiscordMessageAction({
action: "send",
params: {
message: "hello",
},
cfg: {
channels: { discord: { token: "tok" } },
} as OpenClawConfig,
toolContext: {
currentChannelProvider: "discord",
currentChannelId: "channel:123",
},
});
expect(handleDiscordActionMock).toHaveBeenCalledWith(
expect.objectContaining({
action: "sendMessage",
to: "channel:123",
content: "hello",
}),
expect.any(Object),
expect.any(Object),
);
});
it("does not use another provider's current target for Discord sends", async () => {
await expect(
handleDiscordMessageAction({
action: "send",
params: {
message: "hello",
},
cfg: {
channels: { discord: { token: "tok" } },
} as OpenClawConfig,
toolContext: {
currentChannelProvider: "telegram",
currentChannelId: "channel:123",
},
}),
).rejects.toThrow(/channel target is required/i);
expect(handleDiscordActionMock).not.toHaveBeenCalled();
});
it("does not use another provider's current target for Discord reactions", async () => {
await expect(
handleDiscordMessageAction({

View File

@@ -68,7 +68,13 @@ export async function handleDiscordMessageAction(
const resolveChannelId = () => resolveDiscordChannelId(readTarget());
if (action === "send") {
const to = readStringParam(params, "to", { required: true });
const to =
readStringParam(params, "to") ??
readStringParam(params, "target") ??
readCurrentDiscordTarget(ctx.toolContext);
if (!to) {
throw new Error("Discord channel target is required (use channel:<id>).");
}
const asVoice = readBooleanParam(params, "asVoice") === true;
const rawComponents =
buildDiscordPresentationComponents(normalizeMessagePresentation(params.presentation)) ??

View File

@@ -110,6 +110,8 @@ type DispatchInboundParams = {
summary?: string;
title?: string;
}) => Promise<void> | void;
sourceReplyDeliveryMode?: "automatic" | "message_tool_only";
disableBlockStreaming?: boolean;
suppressDefaultToolProgressMessages?: boolean;
onCompactionStart?: () => Promise<void> | void;
onCompactionEnd?: () => Promise<void> | void;
@@ -217,6 +219,30 @@ async function createBaseContext(
return await createBaseDiscordMessageContext(...args);
}
async function createAutomaticSourceDeliveryContext(
overrides: Parameters<typeof createBaseDiscordMessageContext>[0] = {},
): Promise<Awaited<ReturnType<typeof createBaseDiscordMessageContext>>> {
const cfg = (overrides.cfg ?? {}) as {
messages?: {
groupChat?: Record<string, unknown>;
} & Record<string, unknown>;
} & Record<string, unknown>;
return await createBaseContext({
...overrides,
cfg: {
...cfg,
messages: {
...cfg.messages,
ackReaction: cfg.messages?.ackReaction ?? "👀",
groupChat: {
...cfg.messages?.groupChat,
visibleReplies: "automatic",
},
},
},
});
}
function createDirectMessageContextOverrides(
...args: Parameters<typeof createDiscordDirectMessageContextOverrides>
): ReturnType<typeof createDiscordDirectMessageContextOverrides> {
@@ -314,6 +340,12 @@ function getLastDispatchCtx():
return params?.ctx;
}
function getLastDispatchReplyOptions(): DispatchInboundParams["replyOptions"] | undefined {
const callArgs = dispatchInboundMessage.mock.calls.at(-1) as unknown[] | undefined;
const params = callArgs?.[0] as DispatchInboundParams | undefined;
return params?.replyOptions;
}
async function runProcessDiscordMessage(ctx: unknown): Promise<void> {
await processDiscordMessage(ctx as any);
}
@@ -421,7 +453,7 @@ describe("processDiscordMessage ack reactions", () => {
});
it("sends ack reactions for mention-gated guild messages when mentioned", async () => {
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
accountId: "ops",
shouldRequireMention: true,
effectiveWasMentioned: true,
@@ -443,7 +475,7 @@ describe("processDiscordMessage ack reactions", () => {
});
it("uses preflight-resolved messageChannelId when message.channelId is missing", async () => {
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
message: {
id: "m1",
timestamp: new Date().toISOString(),
@@ -482,7 +514,7 @@ describe("processDiscordMessage ack reactions", () => {
return { queuedFinal: true, counts: { final: 1, tool: 0, block: 0 } };
});
const ctx = await createBaseContext();
const ctx = await createAutomaticSourceDeliveryContext();
await runProcessDiscordMessage(ctx);
@@ -503,7 +535,7 @@ describe("processDiscordMessage ack reactions", () => {
return createNoQueuedDispatchResult();
});
const ctx = await createBaseContext();
const ctx = await createAutomaticSourceDeliveryContext();
await processDiscordMessage(ctx as any);
@@ -525,7 +557,7 @@ describe("processDiscordMessage ack reactions", () => {
return createNoQueuedDispatchResult();
});
const ctx = await createBaseContext();
const ctx = await createAutomaticSourceDeliveryContext();
const runPromise = processDiscordMessage(ctx as any);
await vi.advanceTimersByTimeAsync(30_001);
@@ -547,7 +579,7 @@ describe("processDiscordMessage ack reactions", () => {
return createNoQueuedDispatchResult();
});
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
cfg: {
messages: {
ackReaction: "👀",
@@ -573,7 +605,7 @@ describe("processDiscordMessage ack reactions", () => {
return createNoQueuedDispatchResult();
});
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
cfg: {
messages: {
ackReaction: "👀",
@@ -601,7 +633,7 @@ describe("processDiscordMessage ack reactions", () => {
return createNoQueuedDispatchResult();
});
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
cfg: {
messages: {
ackReaction: "👀",
@@ -630,7 +662,7 @@ describe("processDiscordMessage ack reactions", () => {
throw new Error("aborted");
});
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
abortSignal: abortController.signal,
cfg: {
messages: {
@@ -651,7 +683,7 @@ describe("processDiscordMessage ack reactions", () => {
});
it("removes the plain ack reaction when status reactions are disabled and removeAckAfterReply is enabled", async () => {
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
cfg: {
messages: {
ackReaction: "👀",
@@ -753,6 +785,85 @@ describe("processDiscordMessage session routing", () => {
});
});
it("marks always-on guild replies as message-tool-only and disables source streaming", async () => {
const ctx = await createBaseContext({
shouldRequireMention: false,
effectiveWasMentioned: false,
discordConfig: { streaming: "partial", blockStreaming: true },
route: BASE_CHANNEL_ROUTE,
});
await processDiscordMessage(ctx as any);
expect(getLastDispatchReplyOptions()).toMatchObject({
sourceReplyDeliveryMode: "message_tool_only",
disableBlockStreaming: true,
});
expect(createDiscordDraftStream).not.toHaveBeenCalled();
});
it("suppresses automatic status reactions for always-on guild replies", async () => {
const ctx = await createBaseContext({
shouldRequireMention: false,
effectiveWasMentioned: false,
ackReactionScope: "all",
cfg: {
messages: {
ackReaction: "👀",
ackReactionScope: "all",
statusReactions: {
timing: { debounceMs: 0 },
},
},
session: { store: "/tmp/openclaw-discord-process-test-sessions.json" },
},
route: BASE_CHANNEL_ROUTE,
});
await processDiscordMessage(ctx as any);
expect(getLastDispatchReplyOptions()?.sourceReplyDeliveryMode).toBe("message_tool_only");
expect(sendMocks.reactMessageDiscord).not.toHaveBeenCalled();
expect(sendMocks.removeReactionDiscord).not.toHaveBeenCalled();
});
it("defaults guild replies to message-tool-only source delivery", async () => {
await processDiscordMessage(
(await createBaseContext({
shouldRequireMention: true,
effectiveWasMentioned: true,
route: BASE_CHANNEL_ROUTE,
})) as any,
);
expect(getLastDispatchReplyOptions()?.sourceReplyDeliveryMode).toBe("message_tool_only");
dispatchInboundMessage.mockClear();
await processDiscordMessage(
(await createBaseContext({
shouldRequireMention: true,
effectiveWasMentioned: true,
cfg: {
messages: {
groupChat: {
visibleReplies: "automatic",
},
},
session: { store: "/tmp/openclaw-discord-process-test-sessions.json" },
},
route: BASE_CHANNEL_ROUTE,
})) as any,
);
expect(getLastDispatchReplyOptions()?.sourceReplyDeliveryMode).toBe("automatic");
dispatchInboundMessage.mockClear();
await processDiscordMessage(
(await createBaseContext({
...createDirectMessageContextOverrides(),
})) as any,
);
expect(getLastDispatchReplyOptions()?.sourceReplyDeliveryMode).toBeUndefined();
});
it("prefers bound session keys and sets MessageThreadId for bound thread messages", async () => {
const threadBindings = createThreadBindingManager({
cfg: {} as import("openclaw/plugin-sdk/config-types").OpenClawConfig,
@@ -830,7 +941,7 @@ describe("processDiscordMessage draft streaming", () => {
return { queuedFinal: true, counts: { final: 1, tool: 0, block: 0 } };
});
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
discordConfig,
});
@@ -838,7 +949,7 @@ describe("processDiscordMessage draft streaming", () => {
}
async function createBlockModeContext() {
return await createBaseContext({
return await createAutomaticSourceDeliveryContext({
cfg: {
messages: { ackReaction: "👀" },
session: { store: "/tmp/openclaw-discord-process-test-sessions.json" },
@@ -882,7 +993,7 @@ describe("processDiscordMessage draft streaming", () => {
return { queuedFinal: true, counts: { final: 1, tool: 0, block: 0 } };
});
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
cfg: {
messages: { ackReaction: "👀" },
session: { store: "/tmp/openclaw-discord-process-test-sessions.json" },
@@ -917,7 +1028,7 @@ describe("processDiscordMessage draft streaming", () => {
return { queuedFinal: true, counts: { final: 1, tool: 0, block: 0 } };
});
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
discordConfig: { streamMode: "partial", maxLinesPerMessage: 5 },
});
@@ -937,7 +1048,7 @@ describe("processDiscordMessage draft streaming", () => {
return { queuedFinal: true, counts: { final: 1, tool: 0, block: 0 } };
});
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
discordConfig: { streamMode: "partial", maxLinesPerMessage: 5 },
});
@@ -960,7 +1071,7 @@ describe("processDiscordMessage draft streaming", () => {
return { queuedFinal: true, counts: { final: 1, tool: 0, block: 0 } };
});
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
discordConfig: { streamMode: "partial", maxLinesPerMessage: 5 },
});
@@ -989,7 +1100,9 @@ describe("processDiscordMessage draft streaming", () => {
return { queuedFinal: true, counts: { final: 1, tool: 0, block: 0 } };
});
const ctx = await createBaseContext({ discordConfig: { streamMode: "off" } });
const ctx = await createAutomaticSourceDeliveryContext({
discordConfig: { streamMode: "off" },
});
await processDiscordMessage(ctx as any);
@@ -1030,7 +1143,7 @@ describe("processDiscordMessage draft streaming", () => {
return createNoQueuedDispatchResult();
});
const ctx = await createBaseContext({
const ctx = await createAutomaticSourceDeliveryContext({
discordConfig: { streamMode: "partial" },
});

View File

@@ -206,6 +206,12 @@ export async function processDiscordMessage(
if (boundThreadId && typeof threadBindings.touchThread === "function") {
threadBindings.touchThread({ threadId: boundThreadId });
}
const sourceReplyDeliveryMode = isGuildMessage
? cfg.messages?.groupChat?.visibleReplies === "automatic"
? ("automatic" as const)
: ("message_tool_only" as const)
: undefined;
const sourceRepliesAreToolOnly = sourceReplyDeliveryMode === "message_tool_only";
const ackReaction = resolveAckReaction(cfg, route.agentId, {
channel: "discord",
accountId,
@@ -226,7 +232,7 @@ export async function processDiscordMessage(
shouldBypassMention,
}),
);
const shouldSendAckReaction = shouldAckReaction();
const shouldSendAckReaction = !sourceRepliesAreToolOnly && shouldAckReaction();
const statusReactionsEnabled =
shouldSendAckReaction && cfg.messages?.statusReactions?.enabled !== false;
const feedbackRest = createDiscordRestClient({
@@ -607,7 +613,8 @@ export async function processDiscordMessage(
const accountBlockStreamingEnabled =
resolveChannelStreamingBlockEnabled(discordConfig) ??
cfg.agents?.defaults?.blockStreamingDefault === "on";
const canStreamDraft = discordStreamMode !== "off" && !accountBlockStreamingEnabled;
const canStreamDraft =
!sourceRepliesAreToolOnly && discordStreamMode !== "off" && !accountBlockStreamingEnabled;
const draftReplyToMessageId = () => replyReference.peek();
const deliverChannelId = deliverTarget.startsWith("channel:")
? deliverTarget.slice("channel:".length)
@@ -954,11 +961,13 @@ export async function processDiscordMessage(
...replyOptions,
abortSignal,
skillFilter: channelConfig?.skills,
disableBlockStreaming:
disableBlockStreamingForDraft ??
(typeof resolvedBlockStreamingEnabled === "boolean"
? !resolvedBlockStreamingEnabled
: undefined),
sourceReplyDeliveryMode,
disableBlockStreaming: sourceRepliesAreToolOnly
? true
: (disableBlockStreamingForDraft ??
(typeof resolvedBlockStreamingEnabled === "boolean"
? !resolvedBlockStreamingEnabled
: undefined)),
onPartialReply: draftStream ? (payload) => updateDraftFromPartial(payload.text) : undefined,
onAssistantMessageStart: draftStream
? () => {

View File

@@ -173,6 +173,7 @@ describe("monitorSlackProvider tool results", () => {
includeAckReactionConfig?: boolean;
replyToMode?: "off" | "all" | "first";
threadInheritParent?: boolean;
visibleReplies?: "automatic" | "message_tool";
}) {
const slackChannelConfig: Record<string, unknown> = {
dm: { enabled: true, policy: "open", allowFrom: ["*"] },
@@ -187,8 +188,16 @@ describe("monitorSlackProvider tool results", () => {
responsePrefix: "PFX",
ackReaction: "👀",
ackReactionScope: "group-mentions",
...(params.visibleReplies
? { groupChat: { visibleReplies: params.visibleReplies } }
: {}),
}
: { responsePrefix: "PFX" },
: {
responsePrefix: "PFX",
...(params?.visibleReplies
? { groupChat: { visibleReplies: params.visibleReplies } }
: {}),
},
channels: { slack: slackChannelConfig },
...(params?.bindings ? { bindings: params.bindings } : {}),
};
@@ -488,6 +497,9 @@ describe("monitorSlackProvider tool results", () => {
it("accepts channel messages without mention when channels.slack.requireMention is false", async () => {
slackTestState.config = {
messages: {
groupChat: { visibleReplies: "automatic" },
},
channels: {
slack: {
dm: { enabled: true, policy: "open", allowFrom: ["*"] },
@@ -523,6 +535,7 @@ describe("monitorSlackProvider tool results", () => {
includeAckReactionConfig: true,
groupPolicy: "open",
replyToMode: "off",
visibleReplies: "automatic",
});
await runChannelThreadReplyEvent();