Skip to content

fix(feishu): keep group replies in group chats - #24290

Open
feige2003 wants to merge 1 commit into
NousResearch:mainfrom
feige2003:fix/feishu-keep-group-replies-in-group-chats
Open

fix(feishu): keep group replies in group chats#24290
feige2003 wants to merge 1 commit into
NousResearch:mainfrom
feige2003:fix/feishu-keep-group-replies-in-group-chats

Conversation

@feige2003

Copy link
Copy Markdown

What does this PR do?

Fixes Feishu group replies being routed through the reply API even when the inbound message was a normal group @mention rather than a Feishu topic/thread reply.

The root cause was _reply_anchor_for_event() falling back to event.message_id for ordinary Feishu group messages. That caused the adapter to call message.reply(...), which Feishu routes back to the sender DM in this scenario. This change keeps reply semantics only for true Feishu thread/topic replies and lets ordinary group responses go out through the normal group send path.

Related Issue

Fixes #23698
Fixes #23729

Changes Made

  • Updated gateway/platforms/base.py so Feishu only returns a reply anchor for actual thread/topic replies
  • Added a regression test proving non-topic Feishu group sends use the normal group create path instead of message.reply(...)
  • Added a regression test proving ordinary Feishu group events do not request a reply anchor

How to Test

  1. Run uv run --frozen pytest -q -o addopts="" tests/gateway/test_feishu.py::TestFeishuSend::test_send_without_topic_metadata_creates_group_message tests/gateway/test_feishu.py::TestFeishuSend::test_feishu_group_events_do_not_request_reply_anchor
  2. Confirm both tests pass
  3. Run uv run --frozen pytest -q -o addopts="" tests/gateway/test_feishu.py to confirm no regressions

The _reply_anchor_for_event() function was falling back to event.message_id
for ordinary Feishu group messages (those without a thread_id). That caused
the adapter to call message.reply(...) which Feishu routes to the sender's
DM instead of the group chat when the inbound message was a regular group
@mention rather than a Feishu topic/thread reply.

Fix: return None for Feishu non-thread messages so they go through the
normal group create path instead of the reply API.

Fixes NousResearch#23698, NousResearch#23729
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/feishu Feishu / Lark adapter P2 Medium — degraded but workaround exists labels May 12, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #23729 — same fix for the same Feishu group reply routing bug (fixes #23698, #23729). Different author, same approach.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026

@teknium1 teknium1 left a comment

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.

Thanks for isolating the normal-group reply-anchor case. The current main still has the reported control flow: gateway/platforms/base.py:105-107 returns event.message_id for non-threaded Feishu events, and plugins/platforms/feishu/adapter.py:4613-4621 sends any non-empty anchor through Feishu's reply API.

Problems

  • The changed assertion at tests/gateway/test_feishu.py:1998 expects om_reply, but the adapter's unchanged metadata fallback resolves the request target to om_trigger (plugins/platforms/feishu/adapter.py:4609-4621). Current coverage retains om_trigger at tests/gateway/test_feishu.py:2115.
  • The PR predates the Feishu plugin migration (5600105478ffde29d7566b45421b100eaa29c4ef), so its added gateway.platforms.feishu imports and patch targets need to use plugins.platforms.feishu.adapter on current main.

Suggested changes

  • Preserve the existing threaded-topic assertion and transplant the normal-group anchor regression using the current plugin import path.

Automated hermes-sweeper review.

@@ -1998,9 +1998,74 @@ async def _direct(func, *args, **kwargs):
)

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.

This expectation should remain om_trigger: the unchanged adapter fallback uses metadata["reply_to_message_id"] whenever reply_to is absent for a threaded send. The group-anchor change does not alter that target.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feishu: group message replies routed to sender's DM instead of group chat

3 participants