fix(feishu): pass replied-to message attachments through to the agent - #57168
fix(feishu): pass replied-to message attachments through to the agent#57168ll823-png wants to merge 1 commit into
Conversation
Replying to a file/image message and @-mentioning the bot previously handed the agent only a text placeholder like [Attachment: x.pdf] - the parent message's file_key/image_key were parsed and then discarded, so the agent had no way to reach the actual bytes. - _fetch_message_normalized: fetch + cache the referenced message's normalized payload (keeps image_keys/media_refs); _fetch_message_text now derives its text from it. - _fetch_reply_media: download the replied-to message's attachments via the existing message-resource pipeline, cached per parent message_id. - _process_inbound_message: append reply media to the event and, for non-image attachments, prepend a note with the cached file path (the downstream document note only fires for DOCUMENT events). - Empty-text guard now runs after reply-media passthrough so a bare "@bot" reply to an attachment survives instead of being dropped. - Text batching merges media_urls/media_types (deduped) instead of silently dropping them.
alt-glitch
added
type/bug
Competing fix for the Feishu reply-attachment-loss family (fixes #54578) — same problem as the open Feishu reply-media PRs #14371 and #18385, but a distinct/more complete implementation, so relating rather than marking a duplicate. This one caches the referenced message's normalized payload, downloads the replied-to attachments via the message-resource pipeline, and keeps a bare |
|
Thanks @alt-glitch for triaging. To help compare with #14371 (same root cause, similar core approach): this PR additionally ships two new regression tests (reply-media passthrough; bare "@bot" reply surviving the empty-text guard), plus three hardening fixes found in production — media merge in text batching (media was silently dropped when debounced), per-parent media caching, and graceful degradation when the app lacks The fix has been verified end-to-end on a live Feishu tenant (PDF and screenshot reply scenarios). Happy to rebase or fold any of this into #14371 instead if the maintainers prefer that direction. |
teknium1
added
sweeper:risk-compatibility
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying a real Feishu reply-media gap. Current main still fetches only reply_to_text after the empty-text guard in plugins/platforms/feishu/adapter.py:3246-3263; the existing resource path is available at :3751-3804.
Problems
- The patch targets removed
gateway/platforms/feishu.py; Feishu moved toplugins/platforms/feishu/adapter.pyin5600105478ffde29d7566b45421b100eaa29c4ef. - The proposed
_message_context_cacheand_reply_media_cacheare unbounded. Main deliberately caps reply-context caching atplugins/platforms/feishu/adapter.py:4192followinge8cacb57d531137dec3109617e807b30ff5187c9. - A reply-specific non-image note would duplicate current generic document context handling in
gateway/run.py:10648-10690.
Suggested changes
- Port the media lookup before the current guard, use bounded cache lifecycle, and rely on the gateway's existing document note.
- Add a test covering parent fetch/normalization/resource download, not only a mocked
_fetch_reply_media.
Automated hermes-sweeper review.
| self._sent_message_id_order: List[str] = [] # LRU order for _sent_message_ids_to_chat | ||
| self._chat_info_cache: Dict[str, Dict[str, Any]] = {} | ||
| self._message_text_cache: Dict[str, Optional[str]] = {} | ||
| self._message_context_cache: Dict[str, FeishuNormalizedMessage] = {} |
There was a problem hiding this comment.
These new per-parent caches need bounded eviction when ported. Main previously fixed unbounded reply-context caching in e8cacb57d531137dec3109617e807b30ff5187c9; please use a capped LRU policy rather than retaining every referenced message and downloaded attachment for the adapter lifetime.
| if reply_media_urls: | ||
| media_urls = list(media_urls) + reply_media_urls | ||
| media_types = list(media_types) + reply_media_types | ||
| text = self._prepend_reply_attachment_notes(text, reply_media_urls, reply_media_types) |
There was a problem hiding this comment.
On current main, gateway/run.py:10648-10690 already adds a document path note for non-image media even on TEXT events. When porting this logic, pass the media through but avoid adding a second reply-specific note.
4 participants
Problem
On Feishu/Lark, replying to a file or image message and @-mentioning the bot
hands the agent only a text placeholder like
[Replying to: "[Attachment: x.pdf]"].The parent message's
file_key/image_keyare parsed during normalization andthen discarded, so the agent has no way to reach the actual bytes. Users must
re-send the attachment directly in a DM for the bot to see it.
Fix
_fetch_message_normalized: fetch + cache the referenced message'snormalized payload (keeps
image_keys/media_refs);_fetch_message_textnow derives its text from it (behavior unchanged).
_fetch_reply_media: download the replied-to message's attachments via theexisting message-resource pipeline, cached per parent
message_id._process_inbound_message: append reply media to the event; for non-imageattachments, prepend a note with the cached file path (the downstream
document note only fires for DOCUMENT-typed events, not TEXT replies).
"@bot" reply to an attachment survives instead of being dropped.
media_urls/media_types(deduped) instead ofsilently dropping them.
Note: downloading attachments of non-mention group messages requires the app
scope
im:message.group_msg(read all group messages). Without it the fetchfails gracefully (warning log, no media attached) — same as before this patch.
Tests
Two new cases in
tests/gateway/test_feishu.py:path note in text);
empty-text guard.
Verified end-to-end on a live Feishu tenant: "reply to PDF + @bot" and
"reply to screenshot + @bot" both deliver the attachment to the agent.