Skip to content

fix(feishu): avoid threading regular replies - #20562

Open
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix/feishu-root-id-thread-fallback-20548
Open

fix(feishu): avoid threading regular replies#20562
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix/feishu-root-id-thread-fallback-20548

Conversation

@LeonSGP43

Copy link
Copy Markdown
Contributor

Summary

  • stop using Feishu root_id as a fallback thread_id for inbound messages
  • keep root_id available as reply context via reply_to_message_id
  • add regressions for regular replies and explicit thread replies

Fixes #20548

Verification

  • scripts/run_tests.sh tests/gateway/test_feishu.py -k 'ProcessInboundMessage or reply_to_text or thread_metadata'\n- scripts/run_tests.sh tests/gateway/test_feishu.py\n- python -m py_compile gateway/platforms/feishu.py\n- git diff --check

@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 a real Feishu routing problem.

Problems

  • Current main still has the problematic fallback at plugins/platforms/feishu/adapter.py:3252, but this PR changes the pre-plugin-migration path gateway/platforms/feishu.py:2763. A salvage must move the change to the active adapter.
  • The proposed unconditional deletion also removes behavior introduced by merged PR #19711 (a79b0ec46157efc91537e634a3dcc44a76f6dc7e) for Feishu Topic Mode payloads. The PR covers ordinary replies and explicit thread_id, but does not cover the compatibility shape that originally needed root_id.

Suggested changes

  • Resolve inbound routing in the active adapter by preferring explicit thread_id, keeping ordinary quote replies flat, and retaining root_id only for a verified topic-specific fallback.
  • Add regressions for ordinary DM/group replies, explicit topic replies, and the retained fallback case.

Automated hermes-sweeper review.

text = f"{hint}\n\n{text}" if text else hint

thread_id = getattr(message, "thread_id", None) or getattr(message, "root_id", None) or None
thread_id = getattr(message, "thread_id", None) or None

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 removes the fallback for every payload, but merged PR #19711 (a79b0ec46157efc91537e634a3dcc44a76f6dc7e) added it specifically for Feishu Topic Mode. Please distinguish ordinary quote replies from the verified topic-only compatibility shape and add coverage for both.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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 12, 2026
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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feishu] root_id fallback for thread_id causes all replies to be threaded

3 participants