Skip to content

fix(feishu): reply inside thread/topic instead of creating standalone topics#2148

Merged
Hmbown merged 1 commit into
Hmbown:mainfrom
yuanchenglu:fix/feishu-thread-reply
May 26, 2026
Merged

fix(feishu): reply inside thread/topic instead of creating standalone topics#2148
Hmbown merged 1 commit into
Hmbown:mainfrom
yuanchenglu:fix/feishu-thread-reply

Conversation

@yuanchenglu
Copy link
Copy Markdown

@yuanchenglu yuanchenglu commented May 26, 2026

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() in integrations/feishu-bridge/src/index.mjs exclusively used the Lark SDK client.im.message.create() API with only a bare chat_id. The Feishu reply API (client.im.message.reply()) — which keeps messages inside the same thread — was never called.

Fix

Two changes:

1. lib.mjsincomingIdentity()

Added parentId, rootId, threadId fields from the raw Feishu message event, making thread context available to downstream code.

2. index.mjs

  • handleIncomingMessage(): stores the latest incoming messageId as replyToMessageId in the per-chat thread store.
  • sendText(): reads replyToMessageId from the thread store. When present, calls client.im.message.reply() instead of create(). This keeps ALL bot responses nested under the original user message.

Existing chats without replyToMessageId in the store automatically fall back to the old create behavior — 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 messageId as replyToMessageId in the per-chat store and switching sendText to call client.im.message.reply() when that value is present.

  • lib.mjs: incomingIdentity gains three new Feishu thread-context fields (parentId, rootId, threadId), none of which are read by the rest of the PR.
  • index.mjs: handleIncomingMessage writes replyToMessageId into the thread store, and sendText reads it back to choose between the reply and create APIs; however, ensureThread (called for every new chat and for /new//resume commands) replaces the entire chat entry with setChat, silently dropping replyToMessageId before sendText is 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 ensureThread calls setChat with a fresh object whenever it creates a new runtime thread, wiping out the replyToMessageId that handleIncomingMessage just 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 /new and /resume. The feature works as intended only for subsequent messages in an already-threaded chat, which is the minority case. The fallback to create is 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

Filename Overview
integrations/feishu-bridge/src/index.mjs Adds thread-reply logic to sendText and stores replyToMessageId on each incoming message; however ensureThread's setChat call overwrites the replyToMessageId for new chats and after /new or /resume, breaking threading for those cases.
integrations/feishu-bridge/src/lib.mjs Adds parentId, rootId, and threadId fields to incomingIdentity; none of the three new fields is consumed by downstream code in this PR.

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 ✓
Loading

Comments Outside Diff (1)

  1. integrations/feishu-bridge/src/index.mjs, line 261-268 (link)

    P1 setChat in ensureThread erases replyToMessageId

    ensureThread calls threadStore.setChat(chatId, state) with a fresh object that does not include replyToMessageId. This completely overwrites the entry that handleIncomingMessage just wrote, so for every first-ever message in a new chat the store no longer carries a replyToMessageId by the time sendText is called. The same problem occurs after /new (forceNew: true) and /resume (which also calls setChat at line 497). All "Started turn …" confirmations, streaming chunks, and turn-completed messages for those cases fall back to client.im.message.create, creating standalone topics — exactly the behaviour this PR intends to fix.

    Fix in Codex Fix in Claude Code Fix in Cursor

Fix All in Codex Fix All in Claude Code Fix All in Cursor

Reviews (1): Last reviewed commit: "fix(feishu): reply inside thread/topic i..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

… 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 创建新话题。
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +153 to +169
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()
});
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Issues Identified:

  1. State Pollution from Ignored Messages: replyToMessageId is currently updated for every incoming message at the very beginning of handleIncomingMessage(). In group chats, this means any message sent by any user (even those without the bot prefix /ds or from unauthorized users) will overwrite replyToMessageId. When the bot eventually sends a message, it will reply to an unrelated message instead of the original command.
  2. State Overwrite in ensureThread and resumeThread: Both ensureThread() and resumeThread() call threadStore.setChat(), which completely overwrites the chat state and wipes out replyToMessageId. This causes the bot to fall back to creating standalone topics for new or resumed threads.

Solution:

  • Patch threadStore.setChat on the fly to merge the existing state instead of overwriting it.
  • Only update replyToMessageId after 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()
    });
  }

Comment on lines +580 to +590
if (replyMessage) {
await replyMessage({
path: { message_id: replyToMessageId },
data: body
});
} else {
await createMessage({
params: { receive_id_type: "chat_id" },
data: { ...body, receive_id: chatId }
});
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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 }
      });
    }

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +153 to +157
if (identity.messageId) {
const existing = await threadStore.getChat(identity.chatId);
if (existing) {
await threadStore.patchChat(identity.chatId, {
replyToMessageId: identity.messageId,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +585 to +588
} else {
await createMessage({
params: { receive_id_type: "chat_id" },
data: { ...body, receive_id: chatId }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +74 to +76
parentId: message.parent_id || "",
rootId: message.root_id || "",
threadId: message.thread_id || ""
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Fix in Codex Fix in Claude Code Fix in Cursor

@Hmbown Hmbown merged commit 5e53866 into Hmbown:main May 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add SiliconFlow as a built-in provider

2 participants