Skip to content

fix(feishu): keep media replies in threads - #39563

Closed
sweetcornna wants to merge 1 commit into
NousResearch:mainfrom
sweetcornna:fix/39526-feishu-media-thread-reply
Closed

fix(feishu): keep media replies in threads#39563
sweetcornna wants to merge 1 commit into
NousResearch:mainfrom
sweetcornna:fix/39526-feishu-media-thread-reply

Conversation

@sweetcornna

Copy link
Copy Markdown
Contributor

Summary

  • preserve a generic reply_to_message_id in non-Telegram thread metadata
  • apply the fix to both BasePlatformAdapter media delivery and GatewayRunner post-stream media delivery
  • add Feishu image-media regression coverage for both delivery paths

Fixes #39526

Tests

  • .venv\Scripts\python.exe -m pytest tests\gateway\test_tts_media_routing.py tests\gateway\test_telegram_thread_fallback.py::test_base_gateway_metadata_marks_telegram_dm_topics_as_reply_fallback tests\gateway\test_telegram_thread_fallback.py::test_gateway_runner_busy_ack_replies_to_triggering_message_for_telegram_dm_topic tests\gateway\test_telegram_thread_fallback.py::test_media_group_dm_topic_reply_not_found_retry_drops_thread_id -q --timeout-method=thread
  • .venv\Scripts\ruff.exe check gateway\platforms\base.py gateway\run.py tests\gateway\test_tts_media_routing.py
  • .venv\Scripts\python.exe -m py_compile gateway\platforms\base.py gateway\run.py tests\gateway\test_tts_media_routing.py
  • git diff --check (passes with Windows CRLF working-copy warnings only)

Notes

  • tests\gateway\test_feishu.py full-file run on Windows currently fails before assertions when tests clear HOME/LOCALAPPDATA and Path.home() cannot resolve a home directory; not caused by this change.

@sweetcornna
sweetcornna force-pushed the fix/39526-feishu-media-thread-reply branch from d9d8ef4 to b4c2074 Compare June 5, 2026 04:37
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter comp/gateway Gateway runner, session dispatch, delivery labels Jun 5, 2026
@sweetcornna
sweetcornna marked this pull request as ready for review June 5, 2026 05:31
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing both response-media delivery paths; the anchor propagation addresses the current omission in gateway/platforms/base.py:55-77 and gateway/run.py:14357-14401.

Problems

Suggested changes

  • Preserve this response-media anchor work, and pair it with a safe no-anchor fallback in the Feishu adapter that never sends receive_id_type="thread_id"; add a regression for that branch.
  • The proposed helper condition covers every non-Telegram platform. Please scope it to Feishu unless the generic metadata contract is intentional and audited.

Automated hermes-sweeper review.

@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 14, 2026
@tchivs

tchivs commented Jul 14, 2026

Copy link
Copy Markdown

I rebased this PR's commit onto current main with the original authorship preserved, scoped reply-anchor metadata to Feishu per the automated review, and added the missing adapter-side strict no-fallback behavior plus live/topic regression coverage. The consolidated follow-up is #64343. Thank you for the original media-path fix.

@sweetcornna

Copy link
Copy Markdown
Contributor Author

Closing in favor of #64343, which rebases and preserves this PR’s original commit, then completes the adapter-side strict routing and regression coverage requested during review. Thanks for consolidating the Feishu topic fix.

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: MEDIA attachments sent via topic/thread reply land in main conversation instead of thread

4 participants