From 71c2c59c6c471d223d6dbfe923fa72d62f2d6c66 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 22 Feb 2026 14:36:46 -0500 Subject: [PATCH] fix(slack): enforce replyToMode for auto-thread_ts and inline reply tags (#23839) * Slack: respect replyToMode for auto-thread_ts and inline reply tags * Update CHANGELOG.md --- CHANGELOG.md | 1 + src/slack/monitor.tool-result.test.ts | 16 +++++++++- src/slack/monitor/message-handler/dispatch.ts | 2 +- src/slack/monitor/replies.ts | 22 +++++-------- src/slack/threading.test.ts | 32 +++++++++++++++++++ src/slack/threading.ts | 6 +++- 6 files changed, 62 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d160cb67504..820d45272f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Slack/Threading: respect `replyToMode` when Slack auto-populates top-level `thread_ts`, and ignore inline `replyToId` directive tags when `replyToMode` is `off` so thread forcing stays disabled unless explicitly configured. (#23839, #23320, #23513) Thanks @vincentkoc and @dorukardahan. - Slack/Extension: forward `message read` `threadId` to `readMessages` and use delivery-context `threadId` as outbound `thread_ts` fallback so extension replies/reads stay in the correct Slack thread. (#22216, #22485, #23836) Thanks @vincentkoc, @lan17 and @dorukardahan. - Config/Memory: allow `"mistral"` in `agents.defaults.memorySearch.provider` and `agents.defaults.memorySearch.fallback` schema validation. (#14934) Thanks @ThomsenDrake. - Security/Feishu: enforce ID-only allowlist matching for DM/group sender authorization, normalize Feishu ID prefixes during checks, and ignore mutable display names so display-name collisions cannot satisfy allowlist entries. This ships in the next npm release. Thanks @jiseoung for reporting. diff --git a/src/slack/monitor.tool-result.test.ts b/src/slack/monitor.tool-result.test.ts index 3f42ff8a3d3..9ff4f435129 100644 --- a/src/slack/monitor.tool-result.test.ts +++ b/src/slack/monitor.tool-result.test.ts @@ -443,7 +443,7 @@ describe("monitorSlackProvider tool results", () => { expect(sendMock.mock.calls[0][2]).toMatchObject({ threadTs: "111.222" }); }); - it("forces thread replies when replyToId is set", async () => { + it("ignores replyToId directive when replyToMode is off", async () => { replyMock.mockResolvedValue({ text: "forced reply", replyToId: "555" }); slackTestState.config = { messages: { @@ -467,6 +467,20 @@ describe("monitorSlackProvider tool results", () => { }), }); + expect(sendMock).toHaveBeenCalledTimes(1); + expect(sendMock.mock.calls[0][2]).toMatchObject({ threadTs: undefined }); + }); + + it("keeps replyToId directive threading when replyToMode is all", async () => { + replyMock.mockResolvedValue({ text: "forced reply", replyToId: "555" }); + setDirectMessageReplyMode("all"); + + await runSlackMessageOnce(monitorSlackProvider, { + event: makeSlackMessageEvent({ + ts: "789", + }), + }); + expect(sendMock).toHaveBeenCalledTimes(1); expect(sendMock.mock.calls[0][2]).toMatchObject({ threadTs: "555" }); }); diff --git a/src/slack/monitor/message-handler/dispatch.ts b/src/slack/monitor/message-handler/dispatch.ts index d84037e0874..3e0a9136e46 100644 --- a/src/slack/monitor/message-handler/dispatch.ts +++ b/src/slack/monitor/message-handler/dispatch.ts @@ -103,7 +103,6 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag incomingThreadTs, messageTs, hasRepliedRef, - chatType: prepared.isDirectMessage ? "direct" : "channel", isThreadReply, }); @@ -187,6 +186,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag runtime, textLimit: ctx.textLimit, replyThreadTs, + replyToMode: ctx.replyToMode, }); replyPlan.markSent(); }; diff --git a/src/slack/monitor/replies.ts b/src/slack/monitor/replies.ts index 3f92ee332aa..5fa1cb90903 100644 --- a/src/slack/monitor/replies.ts +++ b/src/slack/monitor/replies.ts @@ -16,9 +16,13 @@ export async function deliverReplies(params: { runtime: RuntimeEnv; textLimit: number; replyThreadTs?: string; + replyToMode: "off" | "first" | "all"; }) { for (const payload of params.replies) { - const threadTs = payload.replyToId ?? params.replyThreadTs; + // Keep reply tags opt-in: when replyToMode is off, explicit reply tags + // must not force threading. + const inlineReplyToId = params.replyToMode === "off" ? undefined : payload.replyToId; + const threadTs = inlineReplyToId ?? params.replyThreadTs; const mediaList = payload.mediaUrls ?? (payload.mediaUrl ? [payload.mediaUrl] : []); const text = payload.text ?? ""; if (!text && mediaList.length === 0) { @@ -88,19 +92,11 @@ function createSlackReplyReferencePlanner(params: { incomingThreadTs: string | undefined; messageTs: string | undefined; hasReplied?: boolean; - chatType?: "direct" | "channel" | "group"; isThreadReply?: boolean; }) { - // When already inside a Slack thread, stay in it — but for DMs where the - // "thread" was created by the typing indicator (not a real thread reply), - // respect the user's replyToMode setting. - // See: https://github.com/openclaw/openclaw/issues/16868 - const effectiveMode = - params.chatType === "direct" && !params.isThreadReply - ? params.replyToMode - : params.incomingThreadTs - ? "all" - : params.replyToMode; + // Only force threading for real user thread replies. If Slack auto-populates + // thread_ts on top-level messages, preserve the configured reply mode. + const effectiveMode = params.isThreadReply ? "all" : params.replyToMode; return createReplyReferencePlanner({ replyToMode: effectiveMode, existingId: params.incomingThreadTs, @@ -114,7 +110,6 @@ export function createSlackReplyDeliveryPlan(params: { incomingThreadTs: string | undefined; messageTs: string | undefined; hasRepliedRef: { value: boolean }; - chatType?: "direct" | "channel" | "group"; isThreadReply?: boolean; }): SlackReplyDeliveryPlan { const replyReference = createSlackReplyReferencePlanner({ @@ -122,7 +117,6 @@ export function createSlackReplyDeliveryPlan(params: { incomingThreadTs: params.incomingThreadTs, messageTs: params.messageTs, hasReplied: params.hasRepliedRef.value, - chatType: params.chatType, isThreadReply: params.isThreadReply, }); return { diff --git a/src/slack/threading.test.ts b/src/slack/threading.test.ts index 24e64c122dd..cc519683fb5 100644 --- a/src/slack/threading.test.ts +++ b/src/slack/threading.test.ts @@ -45,6 +45,38 @@ describe("resolveSlackThreadTargets", () => { expect(statusThreadTs).toBeUndefined(); }); + it("does not treat auto-created top-level thread_ts as a real thread when mode is off", () => { + const { replyThreadTs, statusThreadTs, isThreadReply } = resolveSlackThreadTargets({ + replyToMode: "off", + message: { + type: "message", + channel: "C1", + ts: "123", + thread_ts: "123", + }, + }); + + expect(isThreadReply).toBe(false); + expect(replyThreadTs).toBeUndefined(); + expect(statusThreadTs).toBeUndefined(); + }); + + it("keeps first-mode behavior for auto-created top-level thread_ts", () => { + const { replyThreadTs, statusThreadTs, isThreadReply } = resolveSlackThreadTargets({ + replyToMode: "first", + message: { + type: "message", + channel: "C1", + ts: "123", + thread_ts: "123", + }, + }); + + expect(isThreadReply).toBe(false); + expect(replyThreadTs).toBeUndefined(); + expect(statusThreadTs).toBeUndefined(); + }); + it("sets messageThreadId for top-level messages when replyToMode is all", () => { const context = resolveSlackThreadContext({ replyToMode: "all", diff --git a/src/slack/threading.ts b/src/slack/threading.ts index 870999f400c..0a72ffa0f3a 100644 --- a/src/slack/threading.ts +++ b/src/slack/threading.ts @@ -48,7 +48,11 @@ export function resolveSlackThreadTargets(params: { }) { const ctx = resolveSlackThreadContext(params); const { incomingThreadTs, messageTs, isThreadReply } = ctx; - const replyThreadTs = incomingThreadTs ?? (params.replyToMode === "all" ? messageTs : undefined); + const replyThreadTs = isThreadReply + ? incomingThreadTs + : params.replyToMode === "all" + ? messageTs + : undefined; const statusThreadTs = replyThreadTs; return { replyThreadTs, statusThreadTs, isThreadReply }; }