diff --git a/CHANGELOG.md b/CHANGELOG.md index 97e8f1ce2ac..7d51963e131 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,7 @@ Docs: https://docs.openclaw.ai - Slack/HTTP mode startup: treat Slack HTTP accounts as configured when `botToken` + `signingSecret` are present (without requiring `appToken`) in channel config/runtime status so webhook mode is not silently skipped. (#30567) - Slack/Transient request errors: classify Slack request-error messages like `Client network socket disconnected before secure TLS connection was established` as transient in unhandled-rejection fatal detection, preventing temporary network drops from crash-looping the gateway. (#23169) - Slack/Usage footer formatting: wrap session keys in inline code in full response-usage footers so Slack does not parse colon-delimited session segments as emoji shortcodes. (#30258) Thanks @pushkarsingh32. +- Slack/Thread session isolation: route channel/group top-level messages into thread-scoped sessions (`:thread:`) and read inbound `previousTimestamp` from the resolved thread session key, preventing cross-thread context bleed and stale timestamp lookups. (#10686) - Slack/Socket Mode slash startup: treat `app.options()` registration as best-effort and fall back to static arg menus when listener registration fails, preventing Slack monitor startup crash loops on receiver init edge cases. (#21715) - Slack/Legacy streaming config: map boolean `channels.slack.streaming=false` to unified streaming mode `off` (with `nativeStreaming=false`) so legacy configs correctly disable draft preview/native streaming instead of defaulting to `partial`. (#25990) Thanks @chilu18. - Slack/Socket reconnect reliability: reconnect Socket Mode after disconnect/start failures using bounded exponential backoff with abort-aware waits, while preserving clean shutdown behavior and adding disconnect/error helper tests. (#27232) Thanks @pandego. diff --git a/src/slack/monitor/message-handler/prepare.thread-session-key.test.ts b/src/slack/monitor/message-handler/prepare.thread-session-key.test.ts new file mode 100644 index 00000000000..db2e2e6b5ab --- /dev/null +++ b/src/slack/monitor/message-handler/prepare.thread-session-key.test.ts @@ -0,0 +1,142 @@ +import type { App } from "@slack/bolt"; +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../../../config/config.js"; +import type { RuntimeEnv } from "../../../runtime.js"; +import type { ResolvedSlackAccount } from "../../accounts.js"; +import type { SlackMessageEvent } from "../../types.js"; +import { createSlackMonitorContext } from "../context.js"; +import { prepareSlackMessage } from "./prepare.js"; + +function buildCtx(overrides?: { replyToMode?: "all" | "first" | "off" }) { + return createSlackMonitorContext({ + cfg: { + channels: { + slack: { enabled: true, replyToMode: overrides?.replyToMode ?? "all" }, + }, + } as OpenClawConfig, + accountId: "default", + botToken: "token", + app: { client: {} } as App, + runtime: {} as RuntimeEnv, + botUserId: "B1", + teamId: "T1", + apiAppId: "A1", + historyLimit: 0, + sessionScope: "per-sender", + mainKey: "main", + dmEnabled: true, + dmPolicy: "open", + allowFrom: [], + groupDmEnabled: true, + groupDmChannels: [], + defaultRequireMention: false, + groupPolicy: "open", + allowNameMatching: false, + useAccessGroups: false, + reactionMode: "off", + reactionAllowlist: [], + replyToMode: overrides?.replyToMode ?? "all", + threadHistoryScope: "thread", + threadInheritParent: false, + slashCommand: { + enabled: false, + name: "openclaw", + sessionPrefix: "slack:slash", + ephemeral: true, + }, + textLimit: 4000, + ackReactionScope: "group-mentions", + mediaMaxBytes: 1024, + removeAckAfterReply: false, + }); +} + +const account: ResolvedSlackAccount = { + accountId: "default", + enabled: true, + botTokenSource: "config", + appTokenSource: "config", + userTokenSource: "none", + config: {}, +}; + +describe("thread-level session keys", () => { + it("uses thread-level session key for channel messages", async () => { + const ctx = buildCtx(); + ctx.resolveUserName = async () => ({ name: "Alice" }); + + const message: SlackMessageEvent = { + channel: "C123", + channel_type: "channel", + user: "U1", + text: "hello", + ts: "1770408518.451689", + } as SlackMessageEvent; + + const prepared = await prepareSlackMessage({ + ctx, + account, + message, + opts: { source: "message" }, + }); + + expect(prepared).toBeTruthy(); + // Channel messages should get thread-level session key with :thread: suffix + // The resolved session key is in ctxPayload.SessionKey, not route.sessionKey + const sessionKey = prepared!.ctxPayload.SessionKey as string; + expect(sessionKey).toContain(":thread:"); + expect(sessionKey).toContain("1770408518.451689"); + }); + + it("uses parent thread_ts for thread replies", async () => { + const ctx = buildCtx(); + ctx.resolveUserName = async () => ({ name: "Bob" }); + + const message: SlackMessageEvent = { + channel: "C123", + channel_type: "channel", + user: "U2", + text: "reply", + ts: "1770408522.168859", + thread_ts: "1770408518.451689", + } as SlackMessageEvent; + + const prepared = await prepareSlackMessage({ + ctx, + account, + message, + opts: { source: "message" }, + }); + + expect(prepared).toBeTruthy(); + // Thread replies should use the parent thread_ts, not the reply ts + const sessionKey = prepared!.ctxPayload.SessionKey as string; + expect(sessionKey).toContain(":thread:1770408518.451689"); + expect(sessionKey).not.toContain("1770408522.168859"); + }); + + it("does not add thread suffix for DMs", async () => { + const ctx = buildCtx(); + ctx.resolveUserName = async () => ({ name: "Carol" }); + + const message: SlackMessageEvent = { + channel: "D456", + channel_type: "im", + user: "U3", + text: "dm message", + ts: "1770408530.000000", + } as SlackMessageEvent; + + const prepared = await prepareSlackMessage({ + ctx, + account, + message, + opts: { source: "message" }, + }); + + expect(prepared).toBeTruthy(); + // DMs should NOT have :thread: in the session key + const sessionKey = prepared!.ctxPayload.SessionKey as string; + expect(sessionKey).not.toContain(":thread:"); + }); +}); diff --git a/src/slack/monitor/message-handler/prepare.ts b/src/slack/monitor/message-handler/prepare.ts index 79fd7957fda..95134725adc 100644 --- a/src/slack/monitor/message-handler/prepare.ts +++ b/src/slack/monitor/message-handler/prepare.ts @@ -181,19 +181,21 @@ export async function prepareSlackMessage(params: { const threadContext = resolveSlackThreadContext({ message, replyToMode }); const threadTs = threadContext.incomingThreadTs; const isThreadReply = threadContext.isThreadReply; - // When replyToMode="all", every top-level message starts a new thread. - // Use its own ts as threadId so the initial message AND subsequent replies - // in that thread share an isolated session (instead of falling back to the - // base DM/channel session for the first message). + // Keep channel/group sessions thread-scoped to avoid cross-thread context bleed. + // For DMs, preserve existing auto-thread behavior when replyToMode="all". const autoThreadId = !isThreadReply && replyToMode === "all" && threadContext.messageTs ? threadContext.messageTs : undefined; + const canonicalThreadId = isRoomish + ? (threadContext.incomingThreadTs ?? message.ts) + : isThreadReply + ? threadTs + : autoThreadId; const threadKeys = resolveThreadSessionKeys({ baseSessionKey, - threadId: isThreadReply ? threadTs : autoThreadId, - parentSessionKey: - (isThreadReply || autoThreadId) && ctx.threadInheritParent ? baseSessionKey : undefined, + threadId: canonicalThreadId, + parentSessionKey: canonicalThreadId && ctx.threadInheritParent ? baseSessionKey : undefined, }); const sessionKey = threadKeys.sessionKey; const historyKey = @@ -461,7 +463,7 @@ export async function prepareSlackMessage(params: { const envelopeOptions = resolveEnvelopeFormatOptions(ctx.cfg); const previousTimestamp = readSessionUpdatedAt({ storePath, - sessionKey: route.sessionKey, + sessionKey, }); const body = formatInboundEnvelope({ channel: "Slack",