Skip to content

fix(feishu): include attachments from replied messages - #54570

Closed
asdlem wants to merge 2 commits into
NousResearch:mainfrom
asdlem:fix/feishu-attachments
Closed

fix(feishu): include attachments from replied messages#54570
asdlem wants to merge 2 commits into
NousResearch:mainfrom
asdlem:fix/feishu-attachments

Conversation

@asdlem

@asdlem asdlem commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Carries replied attachment media into the current event so the agent can see files/images the user is replying to.

Rebased onto current main (v0.17.0+). Original PR was #24285.

Closes #26037

What this fixes

When replying to a Feishu message that contains attachments (images, documents), the agent only sees the text — the attachments are lost. This PR fetches the parent message's full context including cached attachment resources.

Changes

  • Fetch full Feishu parent-message context for replies
  • Carry replied attachment media into the current event
  • Surface document cache paths for text events with document media

@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: competing open Feishu reply-media PRs — #13115 (earliest, canonical), #27545, #14371. This is a rebase of closed #24285. Verified on main: Feishu reply handling still returns text only (_fetch_message_text), so replied attachments are lost. A maintainer should pick one canonical fix.

@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 addressing the missing parent-media path; the underlying defect remains on current main. Current reply handling calls only _fetch_message_text at plugins/platforms/feishu/adapter.py:3259, while the existing resource pipeline is available at :3771-3800.

Problems

  • On PR head, the empty-text guard at plugins/platforms/feishu/adapter.py:3116-3119 runs before the new lookup at :3137. A pure @Hermes reply to an attachment returns before parent media is hydrated.
  • _fetch_message_context redownloads parent resources on every repeat reply (:4057-4060) and has no context cache. The cache helpers create UUID-named paths (gateway/platforms/base.py:719, :839, :1576).
  • The gateway/run.py hunk is already superseded by current main's broader per-attachment handling at gateway/run.py:10526-10571 (505bc27d8d911465ae1e6e63e9329455f8213ecb).

Suggested changes

  • Hydrate parent context before the empty-text guard and add a bare-mention reply regression test.
  • Add a bounded parent-context LRU cache and tests for repeated-parent reuse.
  • Port the SDK GET through current main's _run_blocking helper (plugins/platforms/feishu/adapter.py:1692-1695) and retain current main's gateway preprocessing.

Automated hermes-sweeper review.

reply_media_urls: List[str] = []
reply_media_types: List[str] = []
if reply_to_message_id:
reply_to_text, reply_media_urls, reply_media_types = await self._fetch_message_context(reply_to_message_id)

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 lookup occurs after the empty-text guard at lines 3116-3119. A user who quote-replies to an attachment with only an @bot mention is stripped to empty and returned before this runs; move hydration before that guard (or make the guard parent-media-aware) and add that regression case.

return None

async def _fetch_message_text(self, message_id: str) -> Optional[str]:
async def _fetch_message_context(self, message_id: str) -> tuple[Optional[str], List[str], List[str]]:

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.

Please add a bounded parent-context cache here. Repeat replies to the same parent re-run the download at line 4057, and the cache helpers create UUID-named files for each download, so repeated replies duplicate cached media until cleanup.

@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 15, 2026
@asdlem

asdlem commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by a clean rebuild from main. See new PR.

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: reply-to-image messages lose parent context (_fetch_message_text returns empty for non-text msgs)

3 participants