From aac83e00cfe72e5395b4cdfe77ead1c534fe7ef8 Mon Sep 17 00:00:00 2001 From: Bek <66288351+bek91@users.noreply.github.com> Date: Mon, 27 Apr 2026 02:19:27 -0400 Subject: [PATCH] fix: Slack inbound thread session routing (#72498) Normalize actionable Slack thread roots and follow-up replies onto the same thread parent session key. --- .../message-handler/prepare-routing.ts | 78 ++- .../message-handler/prepare.test-helpers.ts | 2 +- .../monitor/message-handler/prepare.test.ts | 465 ++++++++++++++++++ .../prepare.thread-session-key.test.ts | 174 ++++++- .../src/monitor/message-handler/prepare.ts | 65 ++- 5 files changed, 741 insertions(+), 43 deletions(-) diff --git a/extensions/slack/src/monitor/message-handler/prepare-routing.ts b/extensions/slack/src/monitor/message-handler/prepare-routing.ts index 7d9a4556270..01a412907e0 100644 --- a/extensions/slack/src/monitor/message-handler/prepare-routing.ts +++ b/extensions/slack/src/monitor/message-handler/prepare-routing.ts @@ -20,6 +20,7 @@ export type SlackRoutingContextDeps = { export type SlackRoutingContext = { route: ReturnType; runtimeBinding: RuntimeConversationBindingRouteResult["bindingRecord"]; + runtimeBoundSessionKey: string | undefined; chatType: "direct" | "group" | "channel"; replyToMode: ReturnType; threadContext: ReturnType; @@ -39,6 +40,25 @@ function resolveSlackBaseConversationId(params: { : params.message.channel; } +function resolveSlackInitialAgentRoute(params: { + ctx: SlackRoutingContextDeps; + account: ResolvedSlackAccount; + message: SlackMessageEvent; + isDirectMessage: boolean; + isRoom: boolean; +}) { + return resolveAgentRoute({ + cfg: params.ctx.cfg, + channel: "slack", + accountId: params.account.accountId, + teamId: params.ctx.teamId || undefined, + peer: { + kind: params.isDirectMessage ? "direct" : params.isRoom ? "channel" : "group", + id: params.isDirectMessage ? (params.message.user ?? "unknown") : params.message.channel, + }, + }); +} + export function resolveSlackRoutingContext(params: { ctx: SlackRoutingContextDeps; account: ResolvedSlackAccount; @@ -47,17 +67,24 @@ export function resolveSlackRoutingContext(params: { isGroupDm: boolean; isRoom: boolean; isRoomish: boolean; + seedTopLevelRoomThread?: boolean; }): SlackRoutingContext { - const { ctx, account, message, isDirectMessage, isGroupDm, isRoom, isRoomish } = params; - let route = resolveAgentRoute({ - cfg: ctx.cfg, - channel: "slack", - accountId: account.accountId, - teamId: ctx.teamId || undefined, - peer: { - kind: isDirectMessage ? "direct" : isRoom ? "channel" : "group", - id: isDirectMessage ? (message.user ?? "unknown") : message.channel, - }, + const { + ctx, + account, + message, + isDirectMessage, + isGroupDm, + isRoom, + isRoomish, + seedTopLevelRoomThread, + } = params; + let route = resolveSlackInitialAgentRoute({ + ctx, + account, + message, + isDirectMessage, + isRoom, }); const chatType = isDirectMessage ? "direct" : isGroupDm ? "group" : "channel"; @@ -72,21 +99,32 @@ export function resolveSlackRoutingContext(params: { !isThreadReply && replyToMode === "all" && threadContext.messageTs ? threadContext.messageTs : undefined; - // Only fork channel/group messages into thread-specific sessions when they are - // actual thread replies (thread_ts present, different from message ts). - // Top-level channel messages must stay on the per-channel session for continuity. - // Before this fix, every channel message used its own ts as threadId, creating - // isolated sessions per message (regression from #10686). + // Keep ordinary top-level room messages on the per-channel session for + // continuity, but preserve Slack thread identity when the event already has + // one or when an actionable app mention will seed a reply thread. + // This keeps a thread root and its later replies on one parent session + // without returning to the old "every channel message is its own thread" + // behavior (regression from #10686). + const seedCandidateThreadId = threadContext.incomingThreadTs ?? threadContext.messageTs; + const seededRoomThreadId = + !isThreadReply && + isRoom && + seedTopLevelRoomThread && + replyToMode !== "off" && + seedCandidateThreadId + ? seedCandidateThreadId + : undefined; const roomThreadId = isThreadReply && threadTs ? threadTs : undefined; const canonicalThreadId = isRoomish ? roomThreadId : isThreadReply ? threadTs : autoThreadId; + const routedThreadId = canonicalThreadId ?? (isRoomish ? seededRoomThreadId : undefined); const baseConversationId = resolveSlackBaseConversationId({ message, isDirectMessage }); - const boundThreadRoute = canonicalThreadId + const boundThreadRoute = routedThreadId ? resolveRuntimeConversationBindingRoute({ route, conversation: { channel: "slack", accountId: account.accountId, - conversationId: canonicalThreadId, + conversationId: routedThreadId, parentConversationId: baseConversationId, }, }) @@ -107,9 +145,8 @@ export function resolveSlackRoutingContext(params: { ? { sessionKey: route.sessionKey, parentSessionKey: undefined } : resolveThreadSessionKeys({ baseSessionKey: route.sessionKey, - threadId: canonicalThreadId, - parentSessionKey: - canonicalThreadId && ctx.threadInheritParent ? route.sessionKey : undefined, + threadId: routedThreadId, + parentSessionKey: routedThreadId && ctx.threadInheritParent ? route.sessionKey : undefined, }); const sessionKey = threadKeys.sessionKey; const historyKey = @@ -118,6 +155,7 @@ export function resolveSlackRoutingContext(params: { return { route, runtimeBinding: runtimeRoute.bindingRecord, + runtimeBoundSessionKey: runtimeRoute.boundSessionKey, chatType, replyToMode, threadContext, diff --git a/extensions/slack/src/monitor/message-handler/prepare.test-helpers.ts b/extensions/slack/src/monitor/message-handler/prepare.test-helpers.ts index c2fd3dcf3f2..60634271d4a 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.test-helpers.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.test-helpers.ts @@ -12,7 +12,7 @@ export function createInboundSlackTestContext(params: { cfg: OpenClawConfig; appClient?: App["client"]; defaultRequireMention?: boolean; - replyToMode?: "off" | "all" | "first"; + replyToMode?: "off" | "all" | "first" | "batched"; channelsConfig?: SlackChannelConfigEntries; threadRequireExplicitMention?: boolean; }) { diff --git a/extensions/slack/src/monitor/message-handler/prepare.test.ts b/extensions/slack/src/monitor/message-handler/prepare.test.ts index 708cc1abfdf..d5e4303b288 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.test.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.test.ts @@ -12,6 +12,10 @@ import { resolveAgentRoute } from "openclaw/plugin-sdk/routing"; import { resolveThreadSessionKeys } from "openclaw/plugin-sdk/routing"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import type { ResolvedSlackAccount } from "../../accounts.js"; +import { + clearSlackThreadParticipationCache, + recordSlackThreadParticipation, +} from "../../sent-thread-cache.js"; import type { SlackMessageEvent } from "../../types.js"; import type { SlackMonitorContext } from "../context.js"; import { resetSlackThreadStarterCacheForTest } from "../thread.js"; @@ -32,6 +36,7 @@ describe("slack prepareSlackMessage inbound contract", () => { beforeEach(() => { resetSlackThreadStarterCacheForTest(); + clearSlackThreadParticipationCache(); }); afterAll(() => { @@ -884,6 +889,466 @@ describe("slack prepareSlackMessage inbound contract", () => { unregisterSessionBindingAdapter({ channel: "slack", accountId: "default", adapter }); } }); + + it("keeps a root app mention and URL-only Slack thread follow-up on one parent session", async () => { + const { storePath } = storeFixture.makeTmpStorePath(); + const rootTs = "1777244692.409919"; + const expectedSessionKey = "agent:main:slack:channel:c0ahzfcas1k:thread:1777244692.409919"; + const replies = vi.fn().mockResolvedValue({ + messages: [ + { + text: "<@B1> send a subagent to review GitHub issue #50621", + user: "U_BEK", + ts: rootTs, + }, + ], + response_metadata: { next_cursor: "" }, + }); + const slackCtx = createInboundSlackCtx({ + cfg: { + session: { store: storePath }, + channels: { slack: { enabled: true, replyToMode: "all", groupPolicy: "open" } }, + } as OpenClawConfig, + appClient: { conversations: { replies } } as unknown as App["client"], + defaultRequireMention: true, + replyToMode: "all", + }); + slackCtx.resolveChannelName = async () => ({ name: "proj-openclaw", type: "channel" }); + slackCtx.resolveUserName = async () => ({ name: "Bek" }); + + const root = await prepareSlackMessage({ + ctx: slackCtx, + account: createSlackAccount({ replyToMode: "all" }), + message: { + type: "message", + channel: "C0AHZFCAS1K", + channel_type: "channel", + user: "U_BEK", + text: "<@B1> send a subagent to review GitHub issue #50621", + ts: rootTs, + } as SlackMessageEvent, + opts: { source: "app_mention", wasMentioned: true }, + }); + recordSlackThreadParticipation("default", "C0AHZFCAS1K", rootTs); + + const followUp = await prepareSlackMessage({ + ctx: slackCtx, + account: createSlackAccount({ replyToMode: "all" }), + message: { + type: "message", + channel: "C0AHZFCAS1K", + channel_type: "channel", + user: "U_BEK", + text: "https://github.com/openclaw/openclaw/issues/50621", + ts: "1777244714.000100", + thread_ts: rootTs, + } as SlackMessageEvent, + opts: { source: "message" }, + }); + + expect(root).toBeTruthy(); + expect(followUp).toBeTruthy(); + expect(root!.ctxPayload.SessionKey).toBe(expectedSessionKey); + expect(followUp!.ctxPayload.SessionKey).toBe(expectedSessionKey); + expect(followUp!.ctxPayload.WasMentioned).toBe(true); + expect(new Set([root!.ctxPayload.SessionKey, followUp!.ctxPayload.SessionKey]).size).toBe(1); + }); + + it("keeps a message-first root mention and URL-only Slack thread follow-up on one parent session", async () => { + const { storePath } = storeFixture.makeTmpStorePath(); + const rootTs = "1777244692.409919"; + const expectedSessionKey = "agent:main:slack:channel:c0ahzfcas1k:thread:1777244692.409919"; + const replies = vi.fn().mockResolvedValue({ + messages: [ + { + text: "<@B1> send a subagent to review GitHub issue #50621", + user: "U_BEK", + ts: rootTs, + }, + ], + response_metadata: { next_cursor: "" }, + }); + const slackCtx = createInboundSlackCtx({ + cfg: { + session: { store: storePath }, + channels: { slack: { enabled: true, replyToMode: "all", groupPolicy: "open" } }, + } as OpenClawConfig, + appClient: { conversations: { replies } } as unknown as App["client"], + defaultRequireMention: true, + replyToMode: "all", + }); + slackCtx.resolveChannelName = async () => ({ name: "proj-openclaw", type: "channel" }); + slackCtx.resolveUserName = async () => ({ name: "Bek" }); + + const root = await prepareSlackMessage({ + ctx: slackCtx, + account: createSlackAccount({ replyToMode: "all" }), + message: { + type: "message", + channel: "C0AHZFCAS1K", + channel_type: "channel", + user: "U_BEK", + text: "<@B1> send a subagent to review GitHub issue #50621", + ts: rootTs, + } as SlackMessageEvent, + opts: { source: "message" }, + }); + recordSlackThreadParticipation("default", "C0AHZFCAS1K", rootTs); + + const followUp = await prepareSlackMessage({ + ctx: slackCtx, + account: createSlackAccount({ replyToMode: "all" }), + message: { + type: "message", + channel: "C0AHZFCAS1K", + channel_type: "channel", + user: "U_BEK", + text: "https://github.com/openclaw/openclaw/issues/50621", + ts: "1777244714.000100", + thread_ts: rootTs, + } as SlackMessageEvent, + opts: { source: "message" }, + }); + + expect(root).toBeTruthy(); + expect(followUp).toBeTruthy(); + expect(root!.ctxPayload.SessionKey).toBe(expectedSessionKey); + expect(followUp!.ctxPayload.SessionKey).toBe(expectedSessionKey); + expect(root!.ctxPayload.WasMentioned).toBe(true); + expect(followUp!.ctxPayload.WasMentioned).toBe(true); + expect(new Set([root!.ctxPayload.SessionKey, followUp!.ctxPayload.SessionKey]).size).toBe(1); + }); + + it("keeps a regex-mentioned Slack thread root and URL-only follow-up on one parent session", async () => { + const { storePath } = storeFixture.makeTmpStorePath(); + const rootTs = "1777244692.409919"; + const expectedSessionKey = "agent:main:slack:channel:c0ahzfcas1k:thread:1777244692.409919"; + const replies = vi.fn().mockResolvedValue({ + messages: [ + { + text: "Bill send a subagent to review GitHub issue #50621", + user: "U_BEK", + ts: rootTs, + }, + ], + response_metadata: { next_cursor: "" }, + }); + const slackCtx = createInboundSlackCtx({ + cfg: { + session: { store: storePath }, + messages: { groupChat: { mentionPatterns: ["\\bbill\\b"] } }, + channels: { slack: { enabled: true, replyToMode: "all", groupPolicy: "open" } }, + } as OpenClawConfig, + appClient: { conversations: { replies } } as unknown as App["client"], + defaultRequireMention: true, + replyToMode: "all", + }); + slackCtx.resolveChannelName = async () => ({ name: "proj-openclaw", type: "channel" }); + slackCtx.resolveUserName = async () => ({ name: "Bek" }); + + const root = await prepareSlackMessage({ + ctx: slackCtx, + account: createSlackAccount({ replyToMode: "all" }), + message: { + type: "message", + channel: "C0AHZFCAS1K", + channel_type: "channel", + user: "U_BEK", + text: "Bill send a subagent to review GitHub issue #50621", + ts: rootTs, + } as SlackMessageEvent, + opts: { source: "message" }, + }); + recordSlackThreadParticipation("default", "C0AHZFCAS1K", rootTs); + + const followUp = await prepareSlackMessage({ + ctx: slackCtx, + account: createSlackAccount({ replyToMode: "all" }), + message: { + type: "message", + channel: "C0AHZFCAS1K", + channel_type: "channel", + user: "U_BEK", + text: "https://github.com/openclaw/openclaw/issues/50621", + ts: "1777244714.000100", + thread_ts: rootTs, + } as SlackMessageEvent, + opts: { source: "message" }, + }); + + expect(root).toBeTruthy(); + expect(followUp).toBeTruthy(); + expect(root!.ctxPayload.SessionKey).toBe(expectedSessionKey); + expect(followUp!.ctxPayload.SessionKey).toBe(expectedSessionKey); + expect(root!.ctxPayload.WasMentioned).toBe(true); + expect(followUp!.ctxPayload.WasMentioned).toBe(true); + }); + + it("keeps runtime-bound regex mentions on the bound parent session", async () => { + const { storePath } = storeFixture.makeTmpStorePath(); + const rootTs = "1777244692.409919"; + const expectedSessionKey = "agent:review:slack:channel:c0ahzfcas1k"; + const binding: SessionBindingRecord = { + bindingId: "slack-review-binding", + targetSessionKey: "agent:review:slack:channel:c0ahzfcas1k", + targetKind: "session", + conversation: { + channel: "slack", + accountId: "default", + conversationId: "C0AHZFCAS1K", + }, + status: "active", + boundAt: 1, + }; + const resolveByConversation = vi.fn((ref) => + ref.conversationId === "C0AHZFCAS1K" ? binding : null, + ); + const adapter: SessionBindingAdapter = { + channel: "slack", + accountId: "default", + listBySession: () => [], + resolveByConversation, + }; + registerSessionBindingAdapter(adapter); + try { + const slackCtx = createInboundSlackCtx({ + cfg: { + session: { store: storePath }, + agents: { + list: [ + { id: "main", default: true }, + { id: "review", groupChat: { mentionPatterns: ["\\breviewbot\\b"] } }, + ], + }, + channels: { slack: { enabled: true, replyToMode: "all", groupPolicy: "open" } }, + } as OpenClawConfig, + defaultRequireMention: true, + replyToMode: "all", + }); + slackCtx.resolveChannelName = async () => ({ name: "proj-openclaw", type: "channel" }); + slackCtx.resolveUserName = async () => ({ name: "Bek" }); + + const prepared = await prepareSlackMessage({ + ctx: slackCtx, + account: createSlackAccount({ replyToMode: "all" }), + message: { + type: "message", + channel: "C0AHZFCAS1K", + channel_type: "channel", + user: "U_BEK", + text: "reviewbot please review GitHub issue #50621", + ts: rootTs, + } as SlackMessageEvent, + opts: { source: "message" }, + }); + recordSlackThreadParticipation("default", "C0AHZFCAS1K", rootTs); + + const followUp = await prepareSlackMessage({ + ctx: slackCtx, + account: createSlackAccount({ replyToMode: "all" }), + message: { + type: "message", + channel: "C0AHZFCAS1K", + channel_type: "channel", + user: "U_BEK", + text: "https://github.com/openclaw/openclaw/issues/50621", + ts: "1777244714.000100", + thread_ts: rootTs, + } as SlackMessageEvent, + opts: { source: "message" }, + }); + + expect(prepared).toBeTruthy(); + expect(followUp).toBeTruthy(); + expect(prepared!.route.agentId).toBe("review"); + expect(prepared!.ctxPayload.SessionKey).toBe(expectedSessionKey); + expect(followUp!.ctxPayload.SessionKey).toBe(expectedSessionKey); + expect(prepared!.ctxPayload.WasMentioned).toBe(true); + expect(followUp!.ctxPayload.WasMentioned).toBe(true); + expect(new Set([prepared!.ctxPayload.SessionKey, followUp!.ctxPayload.SessionKey]).size).toBe( + 1, + ); + } finally { + unregisterSessionBindingAdapter({ channel: "slack", accountId: "default", adapter }); + } + }); + + it("still seeds regex mentions when plugin-owned bindings do not rewrite the route", async () => { + const { storePath } = storeFixture.makeTmpStorePath(); + const rootTs = "1777244692.409919"; + const expectedSessionKey = "agent:main:slack:channel:c0ahzfcas1k:thread:1777244692.409919"; + const binding: SessionBindingRecord = { + bindingId: "plugin-owned-slack-binding", + targetSessionKey: "agent:plugin:slack:channel:c0ahzfcas1k", + targetKind: "session", + conversation: { + channel: "slack", + accountId: "default", + conversationId: "C0AHZFCAS1K", + }, + status: "active", + boundAt: 1, + metadata: { + pluginBindingOwner: "plugin", + pluginId: "demo-plugin", + pluginRoot: "/tmp/demo-plugin", + }, + }; + const resolveByConversation = vi.fn((ref) => + ref.conversationId === "C0AHZFCAS1K" ? binding : null, + ); + const adapter: SessionBindingAdapter = { + channel: "slack", + accountId: "default", + listBySession: () => [], + resolveByConversation, + }; + registerSessionBindingAdapter(adapter); + try { + const slackCtx = createInboundSlackCtx({ + cfg: { + session: { store: storePath }, + messages: { groupChat: { mentionPatterns: ["\\bbill\\b"] } }, + channels: { slack: { enabled: true, replyToMode: "all", groupPolicy: "open" } }, + } as OpenClawConfig, + defaultRequireMention: true, + replyToMode: "all", + }); + slackCtx.resolveChannelName = async () => ({ name: "proj-openclaw", type: "channel" }); + slackCtx.resolveUserName = async () => ({ name: "Bek" }); + + const root = await prepareSlackMessage({ + ctx: slackCtx, + account: createSlackAccount({ replyToMode: "all" }), + message: { + type: "message", + channel: "C0AHZFCAS1K", + channel_type: "channel", + user: "U_BEK", + text: "Bill send a subagent to review GitHub issue #50621", + ts: rootTs, + } as SlackMessageEvent, + opts: { source: "message" }, + }); + recordSlackThreadParticipation("default", "C0AHZFCAS1K", rootTs); + + const followUp = await prepareSlackMessage({ + ctx: slackCtx, + account: createSlackAccount({ replyToMode: "all" }), + message: { + type: "message", + channel: "C0AHZFCAS1K", + channel_type: "channel", + user: "U_BEK", + text: "https://github.com/openclaw/openclaw/issues/50621", + ts: "1777244714.000100", + thread_ts: rootTs, + } as SlackMessageEvent, + opts: { source: "message" }, + }); + + expect(root).toBeTruthy(); + expect(followUp).toBeTruthy(); + expect(root!.route.agentId).toBe("main"); + expect(root!.ctxPayload.SessionKey).toBe(expectedSessionKey); + expect(followUp!.ctxPayload.SessionKey).toBe(expectedSessionKey); + expect(new Set([root!.ctxPayload.SessionKey, followUp!.ctxPayload.SessionKey]).size).toBe(1); + } finally { + unregisterSessionBindingAdapter({ channel: "slack", accountId: "default", adapter }); + } + }); + + it("prepares bare-ping Slack thread replies with the parent thread timestamp", async () => { + const { storePath } = storeFixture.makeTmpStorePath(); + const rootTs = "1777244748.777299"; + const childTs = "1777245202.803289"; + const expectedSessionKey = "agent:main:slack:channel:c0ahzfcas1k:thread:1777244748.777299"; + const childTsSessionKey = "agent:main:slack:channel:c0ahzfcas1k:thread:1777245202.803289"; + const replies = vi.fn().mockResolvedValue({ + messages: [ + { + text: "Original Slack thread root", + user: "U_ROOT", + ts: rootTs, + }, + ], + response_metadata: { next_cursor: "" }, + }); + const slackCtx = createInboundSlackCtx({ + cfg: { + session: { store: storePath }, + channels: { slack: { enabled: true, replyToMode: "all", groupPolicy: "open" } }, + } as OpenClawConfig, + appClient: { conversations: { replies } } as unknown as App["client"], + defaultRequireMention: true, + replyToMode: "all", + }); + slackCtx.resolveChannelName = async () => ({ name: "proj-openclaw", type: "channel" }); + slackCtx.resolveUserName = async () => ({ name: "Bek" }); + + const prepared = await prepareSlackMessage({ + ctx: slackCtx, + account: createSlackAccount({ replyToMode: "all" }), + message: { + type: "message", + channel: "C0AHZFCAS1K", + channel_type: "channel", + user: "U_BEK", + text: "<@B1> ?", + ts: childTs, + thread_ts: rootTs, + parent_user_id: "U_ROOT", + } as SlackMessageEvent, + opts: { source: "message" }, + }); + + expect(prepared).toBeTruthy(); + expect(prepared!.ctxPayload.SessionKey).toBe(expectedSessionKey); + expect(prepared!.ctxPayload.SessionKey).not.toBe(childTsSessionKey); + expect(prepared!.ctxPayload.MessageThreadId).toBe(rootTs); + expect(prepared!.ctxPayload.ReplyToId).toBe(rootTs); + expect(prepared!.ctxPayload.MessageSid).toBe(childTs); + expect(prepared!.ctxPayload.WasMentioned).toBe(true); + }); + + it("preserves single-use reply mode metadata on seeded top-level roots", async () => { + const { storePath } = storeFixture.makeTmpStorePath(); + const rootTs = "1777244692.409919"; + + for (const replyToMode of ["first", "batched"] as const) { + const slackCtx = createInboundSlackCtx({ + cfg: { + session: { store: storePath }, + channels: { slack: { enabled: true, replyToMode, groupPolicy: "open" } }, + } as OpenClawConfig, + defaultRequireMention: true, + replyToMode, + }); + slackCtx.resolveChannelName = async () => ({ name: "proj-openclaw", type: "channel" }); + slackCtx.resolveUserName = async () => ({ name: "Bek" }); + + const prepared = await prepareSlackMessage({ + ctx: slackCtx, + account: createSlackAccount({ replyToMode }), + message: { + type: "message", + channel: "C0AHZFCAS1K", + channel_type: "channel", + user: "U_BEK", + text: "<@B1> send a subagent to review GitHub issue #50621", + ts: rootTs, + } as SlackMessageEvent, + opts: { source: "app_mention", wasMentioned: true }, + }); + + expect(prepared).toBeTruthy(); + expect(prepared!.ctxPayload.SessionKey).toBe( + "agent:main:slack:channel:c0ahzfcas1k:thread:1777244692.409919", + ); + expect(prepared!.ctxPayload.MessageThreadId).toBeUndefined(); + expect(prepared!.ctxPayload.ReplyToId).toBe(rootTs); + } + }); }); describe("prepareSlackMessage sender prefix", () => { diff --git a/extensions/slack/src/monitor/message-handler/prepare.thread-session-key.test.ts b/extensions/slack/src/monitor/message-handler/prepare.thread-session-key.test.ts index 79d3680d25b..f124e6cc64d 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.thread-session-key.test.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.thread-session-key.test.ts @@ -4,7 +4,7 @@ import type { ResolvedSlackAccount } from "../../accounts.js"; import type { SlackMessageEvent } from "../../types.js"; import { resolveSlackRoutingContext, type SlackRoutingContextDeps } from "./prepare-routing.js"; -function buildCtx(overrides?: { replyToMode?: "all" | "first" | "off" }) { +function buildCtx(overrides?: { replyToMode?: "all" | "first" | "off" | "batched" }) { const replyToMode = overrides?.replyToMode ?? "all"; return { cfg: { @@ -18,7 +18,7 @@ function buildCtx(overrides?: { replyToMode?: "all" | "first" | "off" }) { } satisfies SlackRoutingContextDeps; } -function buildAccount(replyToMode: "all" | "first" | "off"): ResolvedSlackAccount { +function buildAccount(replyToMode: "all" | "first" | "off" | "batched"): ResolvedSlackAccount { return { accountId: "default", enabled: true, @@ -97,8 +97,42 @@ describe("thread-level session keys", () => { expect(sessionKey).not.toContain("1770408522.168859"); }); + it("routes actual Slack thread replies by parent thread_ts, not the child message ts", () => { + const ctx = buildCtx({ replyToMode: "all" }); + const account = buildAccount("all"); + const rootTs = "1777244748.777299"; + const childTs = "1777245202.803289"; + + // Slack prepare routing receives Slack's native thread_ts. The persisted + // reply_to_id/topic_id names are derived runtime metadata, not inbound + // fields used by this routing layer. + const routing = resolveSlackRoutingContext({ + ctx, + account, + message: buildChannelMessage({ + channel: "C0AHZFCAS1K", + user: "U_BEK", + text: "<@B1> ?", + ts: childTs, + thread_ts: rootTs, + parent_user_id: "U_ROOT", + }), + isDirectMessage: false, + isGroupDm: false, + isRoom: true, + isRoomish: true, + }); + + const expectedSessionKey = "agent:main:slack:channel:c0ahzfcas1k:thread:1777244748.777299"; + const childTsSessionKey = "agent:main:slack:channel:c0ahzfcas1k:thread:1777245202.803289"; + expect(routing.sessionKey).toBe(expectedSessionKey); + expect(routing.sessionKey).not.toBe(childTsSessionKey); + expect(routing.threadContext.replyToId).toBe(rootTs); + expect(routing.threadContext.messageThreadId).toBe(rootTs); + }); + it("keeps top-level channel messages on the per-channel session regardless of replyToMode", () => { - for (const mode of ["all", "first", "off"] as const) { + for (const mode of ["all", "first", "off", "batched"] as const) { const ctx = buildCtx({ replyToMode: mode }); const account = buildAccount(mode); @@ -128,6 +162,140 @@ describe("thread-level session keys", () => { } }); + it("keeps unseeded top-level room messages with self thread_ts on the channel session", () => { + const ctx = buildCtx({ replyToMode: "off" }); + const account = buildAccount("off"); + + const routing = resolveSlackRoutingContext({ + ctx, + account, + message: buildChannelMessage({ + ts: "1777244692.409919", + thread_ts: "1777244692.409919", + }), + isDirectMessage: false, + isGroupDm: false, + isRoom: true, + isRoomish: true, + }); + + expect(routing.sessionKey).toBe("agent:main:slack:channel:c123"); + }); + + it("does not seed top-level group DM mentions into thread sessions", () => { + const ctx = buildCtx({ replyToMode: "all" }); + const account = buildAccount("all"); + + const routing = resolveSlackRoutingContext({ + ctx, + account, + message: buildChannelMessage({ + channel: "G123", + channel_type: "mpim", + text: "<@B1> send a subagent", + ts: "1777244692.409919", + }), + isDirectMessage: false, + isGroupDm: true, + isRoom: false, + isRoomish: true, + seedTopLevelRoomThread: true, + }); + + expect(routing.sessionKey).toBe("agent:main:slack:group:g123"); + expect(routing.sessionKey).not.toContain(":thread:"); + }); + + it("routes a seeded thread root and replies with the same Slack thread_ts to one parent session", () => { + const ctx = buildCtx({ replyToMode: "all" }); + const account = buildAccount("all"); + const rootTs = "1777244692.409919"; + + const root = resolveSlackRoutingContext({ + ctx, + account, + message: buildChannelMessage({ + channel: "C0AHZFCAS1K", + text: "<@B1> send a subagent to review issue #50621", + ts: rootTs, + thread_ts: rootTs, + }), + isDirectMessage: false, + isGroupDm: false, + isRoom: true, + isRoomish: true, + seedTopLevelRoomThread: true, + }); + const followUp = resolveSlackRoutingContext({ + ctx, + account, + message: buildChannelMessage({ + channel: "C0AHZFCAS1K", + text: "https://github.com/openclaw/openclaw/issues/50621", + ts: "1777244714.000100", + thread_ts: rootTs, + parent_user_id: "U1", + }), + isDirectMessage: false, + isGroupDm: false, + isRoom: true, + isRoomish: true, + }); + + const expectedSessionKey = "agent:main:slack:channel:c0ahzfcas1k:thread:1777244692.409919"; + expect(root.sessionKey).toBe(expectedSessionKey); + expect(followUp.sessionKey).toBe(expectedSessionKey); + expect(root.historyKey).toBe("C0AHZFCAS1K"); + expect(followUp.historyKey).toBe(expectedSessionKey); + expect(new Set([root.sessionKey, followUp.sessionKey]).size).toBe(1); + }); + + it("seeds top-level app mentions into the same parent session used by later thread replies", () => { + const ctx = buildCtx({ replyToMode: "all" }); + const account = buildAccount("all"); + const rootTs = "1777244692.409919"; + + const rootMention = resolveSlackRoutingContext({ + ctx, + account, + message: buildChannelMessage({ + channel: "C0AHZFCAS1K", + text: "<@B1> send a subagent to review issue #50621", + ts: rootTs, + }), + isDirectMessage: false, + isGroupDm: false, + isRoom: true, + isRoomish: true, + seedTopLevelRoomThread: true, + }); + const urlFollowUp = resolveSlackRoutingContext({ + ctx, + account, + message: buildChannelMessage({ + channel: "C0AHZFCAS1K", + text: "https://github.com/openclaw/openclaw/issues/50621", + ts: "1777244714.000100", + thread_ts: rootTs, + }), + isDirectMessage: false, + isGroupDm: false, + isRoom: true, + isRoomish: true, + }); + + const parentSessions = [rootMention.sessionKey, urlFollowUp.sessionKey]; + const spawnedSubagentsByParent = new Set(parentSessions); + + expect(rootMention.sessionKey).toBe(urlFollowUp.sessionKey); + expect(rootMention.sessionKey).toBe( + "agent:main:slack:channel:c0ahzfcas1k:thread:1777244692.409919", + ); + expect(rootMention.historyKey).toBe("C0AHZFCAS1K"); + expect(urlFollowUp.historyKey).toBe(rootMention.sessionKey); + expect(spawnedSubagentsByParent.size).toBe(1); + }); + it("does not add thread suffix for DMs when replyToMode=off", () => { const ctx = buildCtx({ replyToMode: "off" }); const account = buildAccount("off"); diff --git a/extensions/slack/src/monitor/message-handler/prepare.ts b/extensions/slack/src/monitor/message-handler/prepare.ts index c453caba827..0fd87538907 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.ts @@ -282,7 +282,13 @@ export async function prepareSlackMessage(params: { return null; } const { senderId, allowFromLower } = authorization; - const routing = resolveSlackRoutingContext({ + const hasAnyMention = /<@[^>]+>/.test(message.text ?? ""); + const explicitlyMentioned = Boolean( + ctx.botUserId && message.text?.includes(`<@${ctx.botUserId}>`), + ); + const seedTopLevelRoomThreadBySource = + opts.source === "app_mention" || opts.wasMentioned === true || explicitlyMentioned; + let routing = resolveSlackRoutingContext({ ctx, account, message, @@ -290,7 +296,46 @@ export async function prepareSlackMessage(params: { isGroupDm, isRoom, isRoomish, + seedTopLevelRoomThread: seedTopLevelRoomThreadBySource, }); + + const resolveWasMentioned = (mentionRegexes: RegExp[]) => + opts.wasMentioned ?? + (!isDirectMessage && + matchesMentionWithExplicit({ + text: message.text ?? "", + mentionRegexes, + explicit: { + hasAnyMention, + isExplicitlyMentioned: explicitlyMentioned, + canResolveExplicit: Boolean(ctx.botUserId), + }, + })); + let mentionRegexes = resolveCachedMentionRegexes(ctx, routing.route.agentId); + let wasMentioned = resolveWasMentioned(mentionRegexes); + const hasRuntimeBoundSession = Boolean(routing.runtimeBoundSessionKey); + // Runtime bindings already pin the root and later thread replies to the same + // target session, so only unbound regex mentions need a seeded thread reroute. + if ( + !seedTopLevelRoomThreadBySource && + wasMentioned && + isRoom && + !routing.isThreadReply && + !hasRuntimeBoundSession + ) { + routing = resolveSlackRoutingContext({ + ctx, + account, + message, + isDirectMessage, + isGroupDm, + isRoom, + isRoomish, + seedTopLevelRoomThread: true, + }); + mentionRegexes = resolveCachedMentionRegexes(ctx, routing.route.agentId); + wasMentioned = resolveWasMentioned(mentionRegexes); + } const { route, runtimeBinding, @@ -307,24 +352,6 @@ export async function prepareSlackMessage(params: { `slack: routed via bound conversation ${runtimeBinding.conversation.conversationId} -> ${runtimeBinding.targetSessionKey}`, ); } - - const mentionRegexes = resolveCachedMentionRegexes(ctx, route.agentId); - const hasAnyMention = /<@[^>]+>/.test(message.text ?? ""); - const explicitlyMentioned = Boolean( - ctx.botUserId && message.text?.includes(`<@${ctx.botUserId}>`), - ); - const wasMentioned = - opts.wasMentioned ?? - (!isDirectMessage && - matchesMentionWithExplicit({ - text: message.text ?? "", - mentionRegexes, - explicit: { - hasAnyMention, - isExplicitlyMentioned: explicitlyMentioned, - canResolveExplicit: Boolean(ctx.botUserId), - }, - })); const implicitMentionKinds = isDirectMessage || !ctx.botUserId || !message.thread_ts ? []