From f22fc17c78c2ada61fa70b28b0a1f83262166214 Mon Sep 17 00:00:00 2001 From: YolenSong Date: Tue, 3 Mar 2026 07:33:08 +0800 Subject: [PATCH] feat(feishu): prefer thread_id for topic session routing (openclaw#29788) thanks @songyaolun Verified: - pnpm test -- extensions/feishu/src/bot.test.ts extensions/feishu/src/reply-dispatcher.test.ts - pnpm build Co-authored-by: songyaolun <26423459+songyaolun@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> --- CHANGELOG.md | 1 + extensions/feishu/src/bot.test.ts | 212 ++++++++++++++++++ extensions/feishu/src/bot.ts | 170 +++++++++----- extensions/feishu/src/config-schema.ts | 3 + .../feishu/src/reply-dispatcher.test.ts | 24 ++ extensions/feishu/src/reply-dispatcher.ts | 19 +- extensions/feishu/src/types.ts | 1 + 7 files changed, 368 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02a66d73e8a..6c3cb8b0eaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Feishu/topic session routing: use `thread_id` as topic session scope fallback when `root_id` is absent, keep first-turn topic keys stable across thread creation, and force thread replies when inbound events already carry topic/thread context. (#29788) Thanks @songyaolun. - Feishu/DM pairing reply target: send pairing challenge replies to `chat:` instead of `user:` so Lark/Feishu private chats with user-id-only sender payloads receive pairing messages reliably. (#31403) Thanks @stakeswky. - Feishu/Lark private DM routing: treat inbound `chat_type: "private"` as direct-message context for pairing/mention-forward/reaction synthetic handling so Lark private chats behave like Feishu p2p DMs. (#31400) Thanks @stakeswky. - Sandbox/workspace mount permissions: make primary `/workspace` bind mounts read-only whenever `workspaceAccess` is not `rw` (including `none`) across both core sandbox container and sandbox browser create flows. (#32227) Thanks @guanyu-zhang. diff --git a/extensions/feishu/src/bot.test.ts b/extensions/feishu/src/bot.test.ts index 116af803bd7..3247dea9b65 100644 --- a/extensions/feishu/src/bot.test.ts +++ b/extensions/feishu/src/bot.test.ts @@ -1148,6 +1148,83 @@ describe("handleFeishuMessage command authorization", () => { ); }); + it("keeps root_id as topic key when root_id and thread_id both exist", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(false); + + const cfg: ClawdbotConfig = { + channels: { + feishu: { + groups: { + "oc-group": { + requireMention: false, + groupSessionScope: "group_topic_sender", + }, + }, + }, + }, + } as ClawdbotConfig; + + const event: FeishuMessageEvent = { + sender: { sender_id: { open_id: "ou-topic-user" } }, + message: { + message_id: "msg-scope-topic-thread-id", + chat_id: "oc-group", + chat_type: "group", + root_id: "om_root_topic", + thread_id: "omt_topic_1", + message_type: "text", + content: JSON.stringify({ text: "topic sender scope" }), + }, + }; + + await dispatchMessage({ cfg, event }); + + expect(mockResolveAgentRoute).toHaveBeenCalledWith( + expect.objectContaining({ + peer: { kind: "group", id: "oc-group:topic:om_root_topic:sender:ou-topic-user" }, + parentPeer: { kind: "group", id: "oc-group" }, + }), + ); + }); + + it("uses thread_id as topic key when root_id is missing", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(false); + + const cfg: ClawdbotConfig = { + channels: { + feishu: { + groups: { + "oc-group": { + requireMention: false, + groupSessionScope: "group_topic_sender", + }, + }, + }, + }, + } as ClawdbotConfig; + + const event: FeishuMessageEvent = { + sender: { sender_id: { open_id: "ou-topic-user" } }, + message: { + message_id: "msg-scope-topic-thread-only", + chat_id: "oc-group", + chat_type: "group", + thread_id: "omt_topic_1", + message_type: "text", + content: JSON.stringify({ text: "topic sender scope" }), + }, + }; + + await dispatchMessage({ cfg, event }); + + expect(mockResolveAgentRoute).toHaveBeenCalledWith( + expect.objectContaining({ + peer: { kind: "group", id: "oc-group:topic:omt_topic_1:sender:ou-topic-user" }, + parentPeer: { kind: "group", id: "oc-group" }, + }), + ); + }); + it("maps legacy topicSessionMode=enabled to group_topic routing", async () => { mockShouldComputeCommandAuthorized.mockReturnValue(false); @@ -1186,6 +1263,45 @@ describe("handleFeishuMessage command authorization", () => { ); }); + it("maps legacy topicSessionMode=enabled to root_id when both root_id and thread_id exist", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(false); + + const cfg: ClawdbotConfig = { + channels: { + feishu: { + topicSessionMode: "enabled", + groups: { + "oc-group": { + requireMention: false, + }, + }, + }, + }, + } as ClawdbotConfig; + + const event: FeishuMessageEvent = { + sender: { sender_id: { open_id: "ou-legacy-thread-id" } }, + message: { + message_id: "msg-legacy-topic-thread-id", + chat_id: "oc-group", + chat_type: "group", + root_id: "om_root_legacy", + thread_id: "omt_topic_legacy", + message_type: "text", + content: JSON.stringify({ text: "legacy topic mode" }), + }, + }; + + await dispatchMessage({ cfg, event }); + + expect(mockResolveAgentRoute).toHaveBeenCalledWith( + expect.objectContaining({ + peer: { kind: "group", id: "oc-group:topic:om_root_legacy" }, + parentPeer: { kind: "group", id: "oc-group" }, + }), + ); + }); + it("uses message_id as topic root when group_topic + replyInThread and no root_id", async () => { mockShouldComputeCommandAuthorized.mockReturnValue(false); @@ -1224,6 +1340,102 @@ describe("handleFeishuMessage command authorization", () => { ); }); + it("keeps topic session key stable after first turn creates a thread", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(false); + + const cfg: ClawdbotConfig = { + channels: { + feishu: { + groups: { + "oc-group": { + requireMention: false, + groupSessionScope: "group_topic", + replyInThread: "enabled", + }, + }, + }, + }, + } as ClawdbotConfig; + + const firstTurn: FeishuMessageEvent = { + sender: { sender_id: { open_id: "ou-topic-init" } }, + message: { + message_id: "msg-topic-first", + chat_id: "oc-group", + chat_type: "group", + message_type: "text", + content: JSON.stringify({ text: "create topic" }), + }, + }; + const secondTurn: FeishuMessageEvent = { + sender: { sender_id: { open_id: "ou-topic-init" } }, + message: { + message_id: "msg-topic-second", + chat_id: "oc-group", + chat_type: "group", + root_id: "msg-topic-first", + thread_id: "omt_topic_created", + message_type: "text", + content: JSON.stringify({ text: "follow up in same topic" }), + }, + }; + + await dispatchMessage({ cfg, event: firstTurn }); + await dispatchMessage({ cfg, event: secondTurn }); + + expect(mockResolveAgentRoute).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + peer: { kind: "group", id: "oc-group:topic:msg-topic-first" }, + }), + ); + expect(mockResolveAgentRoute).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + peer: { kind: "group", id: "oc-group:topic:msg-topic-first" }, + }), + ); + }); + + it("forces thread replies when inbound message contains thread_id", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(false); + + const cfg: ClawdbotConfig = { + channels: { + feishu: { + groups: { + "oc-group": { + requireMention: false, + groupSessionScope: "group", + replyInThread: "disabled", + }, + }, + }, + }, + } as ClawdbotConfig; + + const event: FeishuMessageEvent = { + sender: { sender_id: { open_id: "ou-thread-reply" } }, + message: { + message_id: "msg-thread-reply", + chat_id: "oc-group", + chat_type: "group", + thread_id: "omt_topic_thread_reply", + message_type: "text", + content: JSON.stringify({ text: "thread content" }), + }, + }; + + await dispatchMessage({ cfg, event }); + + expect(mockCreateFeishuReplyDispatcher).toHaveBeenCalledWith( + expect.objectContaining({ + replyInThread: true, + threadReply: true, + }), + ); + }); + it("does not dispatch twice for the same image message_id (concurrent dedupe)", async () => { mockShouldComputeCommandAuthorized.mockReturnValue(false); diff --git a/extensions/feishu/src/bot.ts b/extensions/feishu/src/bot.ts index c35786d30b3..9bfac4d4d18 100644 --- a/extensions/feishu/src/bot.ts +++ b/extensions/feishu/src/bot.ts @@ -164,6 +164,7 @@ export type FeishuMessageEvent = { message_id: string; root_id?: string; parent_id?: string; + thread_id?: string; chat_id: string; chat_type: "p2p" | "group" | "private"; message_type: string; @@ -193,6 +194,94 @@ export type FeishuBotAddedEvent = { operator_tenant_key?: string; }; +type GroupSessionScope = "group" | "group_sender" | "group_topic" | "group_topic_sender"; + +type ResolvedFeishuGroupSession = { + peerId: string; + parentPeer: { kind: "group"; id: string } | null; + groupSessionScope: GroupSessionScope; + replyInThread: boolean; + threadReply: boolean; +}; + +function resolveFeishuGroupSession(params: { + chatId: string; + senderOpenId: string; + messageId: string; + rootId?: string; + threadId?: string; + groupConfig?: { + groupSessionScope?: GroupSessionScope; + topicSessionMode?: "enabled" | "disabled"; + replyInThread?: "enabled" | "disabled"; + }; + feishuCfg?: { + groupSessionScope?: GroupSessionScope; + topicSessionMode?: "enabled" | "disabled"; + replyInThread?: "enabled" | "disabled"; + }; +}): ResolvedFeishuGroupSession { + const { chatId, senderOpenId, messageId, rootId, threadId, groupConfig, feishuCfg } = params; + + const normalizedThreadId = threadId?.trim(); + const normalizedRootId = rootId?.trim(); + const threadReply = Boolean(normalizedThreadId || normalizedRootId); + const replyInThread = + (groupConfig?.replyInThread ?? feishuCfg?.replyInThread ?? "disabled") === "enabled" || + threadReply; + + const legacyTopicSessionMode = + groupConfig?.topicSessionMode ?? feishuCfg?.topicSessionMode ?? "disabled"; + const groupSessionScope: GroupSessionScope = + groupConfig?.groupSessionScope ?? + feishuCfg?.groupSessionScope ?? + (legacyTopicSessionMode === "enabled" ? "group_topic" : "group"); + + // Keep topic session keys stable across the "first turn creates thread" flow: + // first turn may only have message_id, while the next turn carries root_id/thread_id. + // Prefer root_id first so both turns stay on the same peer key. + const topicScope = + groupSessionScope === "group_topic" || groupSessionScope === "group_topic_sender" + ? (normalizedRootId ?? normalizedThreadId ?? (replyInThread ? messageId : null)) + : null; + + let peerId = chatId; + switch (groupSessionScope) { + case "group_sender": + peerId = `${chatId}:sender:${senderOpenId}`; + break; + case "group_topic": + peerId = topicScope ? `${chatId}:topic:${topicScope}` : chatId; + break; + case "group_topic_sender": + peerId = topicScope + ? `${chatId}:topic:${topicScope}:sender:${senderOpenId}` + : `${chatId}:sender:${senderOpenId}`; + break; + case "group": + default: + peerId = chatId; + break; + } + + const parentPeer = + topicScope && + (groupSessionScope === "group_topic" || groupSessionScope === "group_topic_sender") + ? { + kind: "group" as const, + id: chatId, + } + : null; + + return { + peerId, + parentPeer, + groupSessionScope, + replyInThread, + threadReply, + }; +} + function parseMessageContent(content: string, messageType: string): string { if (messageType === "post") { // Extract text content from rich text post @@ -624,6 +713,7 @@ export function parseFeishuMessageEvent( mentionedBot, rootId: event.message.root_id || undefined, parentId: event.message.parent_id || undefined, + threadId: event.message.thread_id || undefined, content, contentType: event.message.message_type, }; @@ -785,6 +875,18 @@ export async function handleFeishuMessage(params: { const groupConfig = isGroup ? resolveFeishuGroupConfig({ cfg: feishuCfg, groupId: ctx.chatId }) : undefined; + const groupSession = isGroup + ? resolveFeishuGroupSession({ + chatId: ctx.chatId, + senderOpenId: ctx.senderOpenId, + messageId: ctx.messageId, + rootId: ctx.rootId, + threadId: ctx.threadId, + groupConfig, + feishuCfg, + }) + : null; + const groupHistoryKey = isGroup ? (groupSession?.peerId ?? ctx.chatId) : undefined; const dmPolicy = feishuCfg?.dmPolicy ?? "pairing"; const configAllowFrom = feishuCfg?.allowFrom ?? []; const useAccessGroups = cfg.commands?.useAccessGroups !== false; @@ -853,10 +955,10 @@ export async function handleFeishuMessage(params: { log( `feishu[${account.accountId}]: message in group ${ctx.chatId} did not mention bot, recording to history`, ); - if (chatHistories) { + if (chatHistories && groupHistoryKey) { recordPendingHistoryEntryIfEnabled({ historyMap: chatHistories, - historyKey: ctx.chatId, + historyKey: groupHistoryKey, limit: historyLimit, entry: { sender: ctx.senderOpenId, @@ -951,50 +1053,14 @@ export async function handleFeishuMessage(params: { // Using a group-scoped From causes the agent to treat different users as the same person. const feishuFrom = `feishu:${ctx.senderOpenId}`; const feishuTo = isGroup ? `chat:${ctx.chatId}` : `user:${ctx.senderOpenId}`; + const peerId = isGroup ? (groupSession?.peerId ?? ctx.chatId) : ctx.senderOpenId; + const parentPeer = isGroup ? (groupSession?.parentPeer ?? null) : null; + const replyInThread = isGroup ? (groupSession?.replyInThread ?? false) : false; - // Resolve peer ID for session routing. - // Default is one session per group chat; this can be customized with groupSessionScope. - let peerId = isGroup ? ctx.chatId : ctx.senderOpenId; - let groupSessionScope: "group" | "group_sender" | "group_topic" | "group_topic_sender" = - "group"; - let topicRootForSession: string | null = null; - const replyInThread = - isGroup && - (groupConfig?.replyInThread ?? feishuCfg?.replyInThread ?? "disabled") === "enabled"; - - if (isGroup) { - const legacyTopicSessionMode = - groupConfig?.topicSessionMode ?? feishuCfg?.topicSessionMode ?? "disabled"; - groupSessionScope = - groupConfig?.groupSessionScope ?? - feishuCfg?.groupSessionScope ?? - (legacyTopicSessionMode === "enabled" ? "group_topic" : "group"); - - // When topic-scoped sessions are enabled and replyInThread is on, the first - // bot reply creates the thread rooted at the current message ID. - if (groupSessionScope === "group_topic" || groupSessionScope === "group_topic_sender") { - topicRootForSession = ctx.rootId ?? (replyInThread ? ctx.messageId : null); - } - - switch (groupSessionScope) { - case "group_sender": - peerId = `${ctx.chatId}:sender:${ctx.senderOpenId}`; - break; - case "group_topic": - peerId = topicRootForSession ? `${ctx.chatId}:topic:${topicRootForSession}` : ctx.chatId; - break; - case "group_topic_sender": - peerId = topicRootForSession - ? `${ctx.chatId}:topic:${topicRootForSession}:sender:${ctx.senderOpenId}` - : `${ctx.chatId}:sender:${ctx.senderOpenId}`; - break; - case "group": - default: - peerId = ctx.chatId; - break; - } - - log(`feishu[${account.accountId}]: group session scope=${groupSessionScope}, peer=${peerId}`); + if (isGroup && groupSession) { + log( + `feishu[${account.accountId}]: group session scope=${groupSession.groupSessionScope}, peer=${peerId}`, + ); } let route = core.channel.routing.resolveAgentRoute({ @@ -1005,16 +1071,7 @@ export async function handleFeishuMessage(params: { kind: isGroup ? "group" : "direct", id: peerId, }, - // Add parentPeer for binding inheritance in topic-scoped modes. - parentPeer: - isGroup && - topicRootForSession && - (groupSessionScope === "group_topic" || groupSessionScope === "group_topic_sender") - ? { - kind: "group", - id: ctx.chatId, - } - : null, + parentPeer, }); // Dynamic agent creation for DM users @@ -1111,7 +1168,7 @@ export async function handleFeishuMessage(params: { }); let combinedBody = body; - const historyKey = isGroup ? ctx.chatId : undefined; + const historyKey = groupHistoryKey; if (isGroup && historyKey && chatHistories) { combinedBody = buildPendingHistoryContextFromMap({ @@ -1184,6 +1241,7 @@ export async function handleFeishuMessage(params: { skipReplyToInMessages: !isGroup, replyInThread, rootId: ctx.rootId, + threadReply: isGroup ? (groupSession?.threadReply ?? false) : false, mentionTargets: ctx.mentionTargets, accountId: account.accountId, messageCreateTimeMs, diff --git a/extensions/feishu/src/config-schema.ts b/extensions/feishu/src/config-schema.ts index 4b14901b25c..98f90419b4d 100644 --- a/extensions/feishu/src/config-schema.ts +++ b/extensions/feishu/src/config-schema.ts @@ -110,6 +110,9 @@ const GroupSessionScopeSchema = z * Topic session isolation mode for group chats. * - "disabled" (default): All messages in a group share one session * - "enabled": Messages in different topics get separate sessions + * + * Topic routing uses `root_id` when present to keep session continuity and + * falls back to `thread_id` when `root_id` is unavailable. */ const TopicSessionModeSchema = z.enum(["disabled", "enabled"]).optional(); const ReactionNotificationModeSchema = z.enum(["off", "own", "all"]).optional(); diff --git a/extensions/feishu/src/reply-dispatcher.test.ts b/extensions/feishu/src/reply-dispatcher.test.ts index 412bff70c73..4a46a2ee3b6 100644 --- a/extensions/feishu/src/reply-dispatcher.test.ts +++ b/extensions/feishu/src/reply-dispatcher.test.ts @@ -369,6 +369,30 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); }); + it("disables streaming for thread replies and keeps reply metadata", async () => { + createFeishuReplyDispatcher({ + cfg: {} as never, + agentId: "agent", + runtime: { log: vi.fn(), error: vi.fn() } as never, + chatId: "oc_chat", + replyToMessageId: "om_msg", + replyInThread: false, + threadReply: true, + rootId: "om_root_topic", + }); + + const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; + await options.deliver({ text: "```ts\nconst x = 1\n```" }, { kind: "final" }); + + expect(streamingInstances).toHaveLength(0); + expect(sendMarkdownCardFeishuMock).toHaveBeenCalledWith( + expect.objectContaining({ + replyToMessageId: "om_msg", + replyInThread: true, + }), + ); + }); + it("passes replyInThread to media attachments", async () => { createFeishuReplyDispatcher({ cfg: {} as never, diff --git a/extensions/feishu/src/reply-dispatcher.ts b/extensions/feishu/src/reply-dispatcher.ts index 96338c343a8..ffc945513d9 100644 --- a/extensions/feishu/src/reply-dispatcher.ts +++ b/extensions/feishu/src/reply-dispatcher.ts @@ -45,6 +45,8 @@ export type CreateFeishuReplyDispatcherParams = { /** When true, preserve typing indicator on reply target but send messages without reply metadata */ skipReplyToInMessages?: boolean; replyInThread?: boolean; + /** True when inbound message is already inside a thread/topic context */ + threadReply?: boolean; rootId?: string; mentionTargets?: MentionTarget[]; accountId?: string; @@ -62,11 +64,14 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP replyToMessageId, skipReplyToInMessages, replyInThread, + threadReply, rootId, mentionTargets, accountId, } = params; const sendReplyToMessageId = skipReplyToInMessages ? undefined : replyToMessageId; + const threadReplyMode = threadReply === true; + const effectiveReplyInThread = threadReplyMode ? true : replyInThread; const account = resolveFeishuAccount({ cfg, accountId }); const prefixContext = createReplyPrefixContext({ cfg, agentId }); @@ -125,7 +130,9 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP const chunkMode = core.channel.text.resolveChunkMode(cfg, "feishu"); const tableMode = core.channel.text.resolveMarkdownTableMode({ cfg, channel: "feishu" }); const renderMode = account.config?.renderMode ?? "auto"; - const streamingEnabled = account.config?.streaming !== false && renderMode !== "raw"; + // Card streaming may miss thread affinity in topic contexts; use direct replies there. + const streamingEnabled = + !threadReplyMode && account.config?.streaming !== false && renderMode !== "raw"; let streaming: FeishuStreamingSession | null = null; let streamText = ""; @@ -152,7 +159,7 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP try { await streaming.start(chatId, resolveReceiveIdType(chatId), { replyToMessageId, - replyInThread, + replyInThread: effectiveReplyInThread, rootId, }); } catch (error) { @@ -235,7 +242,7 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP to: chatId, mediaUrl, replyToMessageId: sendReplyToMessageId, - replyInThread, + replyInThread: effectiveReplyInThread, accountId, }); } @@ -255,7 +262,7 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP to: chatId, text: chunk, replyToMessageId: sendReplyToMessageId, - replyInThread, + replyInThread: effectiveReplyInThread, mentions: first ? mentionTargets : undefined, accountId, }); @@ -273,7 +280,7 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP to: chatId, text: chunk, replyToMessageId: sendReplyToMessageId, - replyInThread, + replyInThread: effectiveReplyInThread, mentions: first ? mentionTargets : undefined, accountId, }); @@ -289,7 +296,7 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP to: chatId, mediaUrl, replyToMessageId: sendReplyToMessageId, - replyInThread, + replyInThread: effectiveReplyInThread, accountId, }); } diff --git a/extensions/feishu/src/types.ts b/extensions/feishu/src/types.ts index aae6f6feae7..796fbbbebc6 100644 --- a/extensions/feishu/src/types.ts +++ b/extensions/feishu/src/types.ts @@ -40,6 +40,7 @@ export type FeishuMessageContext = { mentionedBot: boolean; rootId?: string; parentId?: string; + threadId?: string; content: string; contentType: string; /** Mention forward targets (excluding the bot itself) */