fix(feishu): reply inside thread/topic instead of creating standalone topics#2148
fix(feishu): reply inside thread/topic instead of creating standalone topics#2148yuanchenglu wants to merge 1 commit into
Conversation
… topics
When running in a Feishu thread-enabled group (话题群), every bot
response — status messages, approval prompts, streaming progress,
turn results — was sent via the Lark SDK's `create` API which spawns
a new standalone topic. The user sees a cluttered group with orphan
topics for each intermediate bot message.
Root cause: `sendText()` only called `client.im.message.create()`
with a bare `chat_id`, never passing any reply context. The Feishu
`reply` API was completely unused.
Fix (two changes, one site each):
1. **lib.mjs — incomingIdentity()**: expose `parentId`, `rootId`,
`threadId` from the raw Feishu message event so callers can
determine thread context. (Not consumed directly yet, but
available for future use.)
2. **index.mjs**:
- `handleIncomingMessage()`: store the latest incoming
`messageId` as `replyToMessageId` in the per-chat thread store.
- `sendText()`: look up `replyToMessageId` from the thread store;
when present, call `client.im.message.reply()` instead of
`create()`. This keeps ALL bot responses nested under the
original user message inside the same topic.
No config changes needed. New chats automatically start using the
reply path; existing chats without a `replyToMessageId` in the store
fall back to the old `create` behaviour.
/ 修复飞书话题群中 bot 消息新建独立话题的问题。所有回复改为使用 reply API
/ 在原话题内嵌套回复,而非通过 create API 创建新话题。
There was a problem hiding this comment.
Code Review
This pull request introduces changes to the Feishu bridge integration to store incoming message IDs and reply within the same thread or topic instead of spawning new standalone topics. The review feedback highlights two main issues: first, updating replyToMessageId on every incoming message can cause state pollution and incorrect threading in group chats, especially when messages are overwritten by subsequent setChat calls; second, if the reply API fails (e.g., due to a deleted message), the bot's response will fail entirely. Actionable suggestions are provided to validate incoming messages before updating the state and to implement a fallback to the standard message creation API if a reply fails.
| if (identity.messageId) { | ||
| const existing = await threadStore.getChat(identity.chatId); | ||
| if (existing) { | ||
| await threadStore.patchChat(identity.chatId, { | ||
| replyToMessageId: identity.messageId, | ||
| updatedAt: new Date().toISOString() | ||
| }); | ||
| } else { | ||
| await threadStore.setChat(identity.chatId, { | ||
| replyToMessageId: identity.messageId, | ||
| threadId: null, | ||
| lastSeq: 0, | ||
| activeTurnId: null, | ||
| updatedAt: new Date().toISOString() | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Issues Identified:
- State Pollution from Ignored Messages:
replyToMessageIdis currently updated for every incoming message at the very beginning ofhandleIncomingMessage(). In group chats, this means any message sent by any user (even those without the bot prefix/dsor from unauthorized users) will overwritereplyToMessageId. When the bot eventually sends a message, it will reply to an unrelated message instead of the original command. - State Overwrite in
ensureThreadandresumeThread: BothensureThread()andresumeThread()callthreadStore.setChat(), which completely overwrites the chat state and wipes outreplyToMessageId. This causes the bot to fall back to creating standalone topics for new or resumed threads.
Solution:
- Patch
threadStore.setChaton the fly to merge the existing state instead of overwriting it. - Only update
replyToMessageIdafter validating that the message is a text message, matches the group prefix, and is from an allowed user/group.
if (!threadStore._patched) {
threadStore._patched = true;
const originalSetChat = threadStore.setChat.bind(threadStore);
threadStore.setChat = async (chatId, state) => {
const existing = await threadStore.getChat(chatId);
return originalSetChat(chatId, { ...existing, ...state });
};
}
const isTextMessage = !identity.messageType || identity.messageType === "text";
const rawText = isTextMessage ? parseTextContent(event.message?.content || "") : "";
const scoped = stripGroupPrefix(rawText, {
chatType: identity.chatType,
requirePrefix: config.requirePrefixInGroup,
prefix: config.groupPrefix
});
const isGroupAllowed = identity.chatType === "p2p" || config.allowGroups;
const isUserAllowed = isAllowed(identity, config.allowlist, config.allowUnlisted);
if (identity.messageId && isTextMessage && scoped.accepted && isGroupAllowed && isUserAllowed) {
await threadStore.patchChat(identity.chatId, {
replyToMessageId: identity.messageId,
updatedAt: new Date().toISOString()
});
}| if (replyMessage) { | ||
| await replyMessage({ | ||
| path: { message_id: replyToMessageId }, | ||
| data: body | ||
| }); | ||
| } else { | ||
| await createMessage({ | ||
| params: { receive_id_type: "chat_id" }, | ||
| data: { ...body, receive_id: chatId } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Issue Identified:
If replyMessage fails (for example, if the message we are replying to was deleted, or if the message ID is invalid/expired), it will throw an error, and the bot won't be able to send the message at all.
Solution:
Wrap the replyMessage call in a try-catch block and fall back to createMessage so that the bot's response is still delivered to the chat.
if (replyMessage) {
try {
await replyMessage({
path: { message_id: replyToMessageId },
data: body
});
} catch (error) {
console.error("Failed to reply to message " + replyToMessageId + ", falling back to createMessage:", error);
await createMessage({
params: { receive_id_type: "chat_id" },
data: { ...body, receive_id: chatId }
});
}
} else {
await createMessage({
params: { receive_id_type: "chat_id" },
data: { ...body, receive_id: chatId }
});
}There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0b82b6ec0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (identity.messageId) { | ||
| const existing = await threadStore.getChat(identity.chatId); | ||
| if (existing) { | ||
| await threadStore.patchChat(identity.chatId, { | ||
| replyToMessageId: identity.messageId, |
There was a problem hiding this comment.
Scope reply target to active command message
This updates replyToMessageId for every inbound message before any filtering, so while a turn is streaming, any unrelated message in the same group can overwrite the target and cause later sendText() calls to reply under the wrong topic/message. In a thread-enabled chat with concurrent chatter, progress/final outputs can jump away from the original command thread, which breaks the intended conversation grouping.
Useful? React with 👍 / 👎.
| } else { | ||
| await createMessage({ | ||
| params: { receive_id_type: "chat_id" }, | ||
| data: { ...body, receive_id: chatId } |
There was a problem hiding this comment.
Preserve reply target across chat state resets
sendText() falls back to create() whenever replyToMessageId is missing, which creates standalone topics again. Because other paths in this file (ensureThread()/resumeThread()) still replace chat state via setChat(...) without carrying replyToMessageId, the bridge can drop the reply target right before status messages like "Started turn...", so new/resumed conversations regress to the old behavior this commit is trying to fix.
Useful? React with 👍 / 👎.
| parentId: message.parent_id || "", | ||
| rootId: message.root_id || "", | ||
| threadId: message.thread_id || "" |
There was a problem hiding this comment.
threadId / parentId / rootId are never consumed
The three new fields (parentId, rootId, threadId) are populated in incomingIdentity but none of them is read anywhere in index.mjs. The threading logic in handleIncomingMessage exclusively stores identity.messageId as replyToMessageId. If these fields are intended for future use, a comment noting that would help; if they are not needed now, omitting them avoids confusion with the runtime threadId already stored in ThreadStore.
Summary
When running in a Feishu thread-enabled group (话题群), every bot response — status messages, approval prompts, streaming progress, turn results — creates a new standalone topic instead of staying under the original user message.
Before: User sees a cluttered group with orphan topics for every intermediate bot message ("Started turn xxx", approval requests, progress updates, etc.).
After: All bot responses are nested under the original user message inside the same topic, keeping the group clean.
Root Cause
sendText()inintegrations/feishu-bridge/src/index.mjsexclusively used the Lark SDKclient.im.message.create()API with only a barechat_id. The FeishureplyAPI (client.im.message.reply()) — which keeps messages inside the same thread — was never called.Fix
Two changes:
1.
lib.mjs—incomingIdentity()Added
parentId,rootId,threadIdfields from the raw Feishu message event, making thread context available to downstream code.2.
index.mjshandleIncomingMessage(): stores the latest incomingmessageIdasreplyToMessageIdin the per-chat thread store.sendText(): readsreplyToMessageIdfrom the thread store. When present, callsclient.im.message.reply()instead ofcreate(). This keeps ALL bot responses nested under the original user message.Existing chats without
replyToMessageIdin the store automatically fall back to the oldcreatebehavior — no data loss or breaking changes.Testing
Manually verified by sending messages in a Feishu thread-enabled group and observing that all bot responses now appear inside the same topic thread.
Closes #1710
Greptile Summary
This PR fixes Feishu thread-enabled groups ("话题群") where every bot response was creating a new standalone topic instead of staying nested under the original user message. It achieves this by storing the incoming
messageIdasreplyToMessageIdin the per-chat store and switchingsendTextto callclient.im.message.reply()when that value is present.lib.mjs:incomingIdentitygains three new Feishu thread-context fields (parentId,rootId,threadId), none of which are read by the rest of the PR.index.mjs:handleIncomingMessagewritesreplyToMessageIdinto the thread store, andsendTextreads it back to choose between the reply and create APIs; however,ensureThread(called for every new chat and for/new//resumecommands) replaces the entire chat entry withsetChat, silently droppingreplyToMessageIdbeforesendTextis invoked.Confidence Score: 3/5
The reply-threading fix works correctly for repeat messages in established chats, but silently fails for the most common new-user path and after /new and /resume commands.
The central problem is that
ensureThreadcallssetChatwith a fresh object whenever it creates a new runtime thread, wiping out thereplyToMessageIdthathandleIncomingMessagejust wrote. This means every first-ever message in a brand-new chat — the primary scenario a new user would encounter — still produces standalone Feishu topics instead of nested replies. The same wipe happens after/newand/resume. The feature works as intended only for subsequent messages in an already-threaded chat, which is the minority case. The fallback tocreateis graceful (no crash, no data loss), but the stated goal of the PR is not fully achieved.integrations/feishu-bridge/src/index.mjs — specifically the setChat call inside ensureThread (line ~261) and the matching call inside resumeThread (line ~497).
Important Files Changed
Sequence Diagram
sequenceDiagram participant U as Feishu User participant H as handleIncomingMessage participant TS as ThreadStore participant ET as ensureThread participant ST as sendText participant Lark as Lark SDK U->>H: first message (new chat) H->>TS: "setChat(chatId, {replyToMessageId: msgId, threadId: null, ...})" Note over TS: replyToMessageId stored ✓ H->>ET: ensureThread(chatId) ET->>TS: "getChat(chatId) → {threadId: null, ...}" ET->>ET: threadId is null → create runtime thread ET->>TS: "setChat(chatId, {threadId: runtimeId, ...})" Note over TS: replyToMessageId LOST ✗ ET-->>H: state (no replyToMessageId) H->>ST: sendText("Started turn X") ST->>TS: getChat(chatId) → no replyToMessageId ST->>Lark: im.message.create() ← standalone topic U->>H: second message (existing chat) H->>TS: "patchChat(chatId, {replyToMessageId: msg2Id})" Note over TS: replyToMessageId stored ✓ H->>ET: ensureThread(chatId) ET->>TS: "getChat(chatId) → {threadId: runtimeId, ...}" Note over ET: threadId present → early return, no setChat H->>ST: sendText("Started turn Y") ST->>TS: getChat(chatId) → replyToMessageId present ST->>Lark: im.message.reply() ← stays in thread ✓Comments Outside Diff (1)
integrations/feishu-bridge/src/index.mjs, line 261-268 (link)setChatinensureThreaderasesreplyToMessageIdensureThreadcallsthreadStore.setChat(chatId, state)with a fresh object that does not includereplyToMessageId. This completely overwrites the entry thathandleIncomingMessagejust wrote, so for every first-ever message in a new chat the store no longer carries areplyToMessageIdby the timesendTextis called. The same problem occurs after/new(forceNew: true) and/resume(which also callssetChatat line 497). All "Started turn …" confirmations, streaming chunks, and turn-completed messages for those cases fall back toclient.im.message.create, creating standalone topics — exactly the behaviour this PR intends to fix.Reviews (1): Last reviewed commit: "fix(feishu): reply inside thread/topic i..." | Re-trigger Greptile