Skip to content

fix(gateway): preserve reply routing for streamed media sends - #15280

Open
shamork wants to merge 3 commits into
NousResearch:mainfrom
shamork:fix/feishu-streamed-media-reply-routing
Open

shamork wants to merge 3 commits into
NousResearch:mainfrom
shamork:fix/feishu-streamed-media-reply-routing

Conversation

@shamork

@shamork shamork commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Test Plan

  • scripts/run_tests.sh tests/gateway/test_run_progress_topics.py::test_post_stream_media_file_replies_to_originating_feishu_message

Notes

  • this branch intentionally includes the gateway reply-routing dependency so it can be reviewed in sequence

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/feishu Feishu / Lark adapter labels Apr 24, 2026
@shamork
shamork force-pushed the fix/feishu-streamed-media-reply-routing branch from b34e09e to e5049eb Compare April 27, 2026 08:31

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for tracing the post-stream attachment path. The underlying Feishu reply-routing gap remains on current main: gateway/run.py:13134-13211 builds thread metadata but does not pass a reply anchor to attachment sends, while plugins/platforms/feishu/adapter.py:4609-4621 uses that anchor to send a threaded reply.

Problems

  • The proposed _build_stream_reply_routing() returns event_message_id as reply_to for every platform. Current gateway/platforms/base.py:87-107 intentionally suppresses reply anchors for Telegram forum topics; salvage should use _reply_anchor_for_event(event) instead.
  • The patch targets the removed gateway/platforms/feishu.py and a superseded stream-consumer constructor. Current main uses plugins/platforms/feishu/adapter.py and initial_reply_to_id at gateway/run.py:16654-16662.
  • Current post-stream images go through send_multiple_images() (gateway/run.py:13162-13169), whose base signature lacks reply_to (gateway/platforms/base.py:3182-3237); the current image path needs explicit coverage.

Suggested changes

  • Port the attachment-only fix to current _deliver_media_from_response() using the established platform-aware anchor helper, and cover document/audio/video plus batched-image delivery for a threaded Feishu event.

Automated hermes-sweeper review.

Comment thread gateway/run.py
platforms should only use explicit source.thread_id metadata.
"""
if source.platform == Platform.SLACK:
thread_id = source.thread_id or event_message_id

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This returns event_message_id as reply_to for every platform. Current main deliberately returns no reply anchor for Telegram forum topics (gateway/platforms/base.py:87-107); salvage this through _reply_anchor_for_event(event) rather than a generic fallback.

@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-broad Sweeper blast radius: broad — a core path most sessions hit area/streaming Streaming responses: gateway delivery, provider wire labels Jul 12, 2026

@GottZ GottZ 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.

This was generated by AI during triage.

Summary

Three PRs address the reply-routing complex. #15258 and #15280 both propagate the originating reply anchor through streamed/mid-turn sends and post-stream voice, video, image, and document delivery, while #15279 contains nearly the same routing groundwork but omits the post-stream attachment path that remains missing on current main.

Related pull requests

  • #15258 [closed] related — (+576/-31) — superseded candidate: Although closed, its diff remains relevant because it includes the same post-stream attachment routing as #15280, alongside broader progress, commentary, approval, retry, and stream-consumer propagation; it also contains an unrelated Feishu group-mention configuration change.
  • #15279 [closed] related — (+520/-34) — implemented on main: This closed PR propagates reply anchors through streamed and mid-turn text paths, but not through _deliver_media_from_response(); the contributor closure review identifies commits ff14666 and 441ef75 as the current-main implementation of that shared groundwork.
  • #15280 related — (+587/-35) — salvage and update: Its materially useful residual is post-stream attachment routing, but that change is also present in closed #15258 rather than unique to this PR. Consistent with the keep_open review on #15280, the fix should be ported onto current main using _reply_anchor_for_event(event), the plugin-based Feishu adapter, and the current initial_reply_to_id API, with explicit send_multiple_images coverage; the present diff targets removed or superseded interfaces and applies event_message_id too broadly across platforms.

Duplicates

#15258 and #15280 contain essentially the same full reply-routing and post-stream attachment changes; #15279 duplicates their shared streamed/mid-turn routing groundwork but lacks the attachment-delivery additions.

Suggested consolidation

Merge #15280 only after reducing it to a current-main port of the remaining post-stream media/file gap and addressing every item in its keep_open review, especially platform-safe reply-anchor selection and the current multi-image path. Keep #15258 and #15279 closed as superseded/implemented-on-main duplicates; do not merge any of the stale full diffs as-is.

Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 116 kB of PR diffs, 2 kB of issue/PR text, 3 kB of discussion (2 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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

Labels

area/streaming Streaming responses: gateway delivery, provider wire comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit 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.

4 participants