Skip to content

fix(feishu): reply-to media injection for image-only parent messages - #29641

Open
rpplusplus wants to merge 1 commit into
NousResearch:mainfrom
rpplusplus:fix/feishu-reply-image-media
Open

fix(feishu): reply-to media injection for image-only parent messages#29641
rpplusplus wants to merge 1 commit into
NousResearch:mainfrom
rpplusplus:fix/feishu-reply-image-media

Conversation

@rpplusplus

Copy link
Copy Markdown

Problem

When a user replies to an image-only message (no text), the agent sees [Replying to: "None"] and cannot see the original image.

Root Cause Chain

  1. _fetch_message_text() calls _extract_text_from_raw_content()
  2. For image messages, text_content="" and metadata=None
  3. str(None).strip() returns "None" (the string), not Python None
  4. "None" or None evaluates to "None" (truthy string)
  5. The string "None" is cached and returned as reply_to_text
  6. Media from the parent message is never fetched

Logs Before Fix

2026-05-21 10:46:21,487 INFO gateway.platforms.feishu: [Feishu] Reply context: message_id=om_x100b6fd351011080c2959095c8c8244 parent_id=om_x100b6fd21f2670b4c10f098ad1a8987 ... text='None'

Fix

  • Rename _fetch_message_text_fetch_reply_context
  • Return (text, media_urls, media_types) tuple instead of just text
  • Download images/files from parent message and inject into current message's media
  • Fix str(None).strip() or Nonestr(placeholder).strip() if placeholder is not None else None
  • When parent has media but no text, set reply_to_text = "(image)"
  • Add debug logging for media download operations
  • Update all tests to match new API

What does this PR do?

Fixes a bug where replying to image-only messages in Feishu showed [Replying to: "None"] instead of passing the original image to the agent. The root cause was a Python type coercion issue where str(None) returns the string "None", which is truthy, causing it to be used as the reply text. This PR also adds media downloading from parent messages so the agent can see images being replied to.

Related Issue

Fixes the issue described in feishu-platform skill: "Reply-to for image-only messages returns 'None'"

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/platforms/feishu.py: Renamed _fetch_message_text to _fetch_reply_context, returns (text, media_urls, media_types) tuple, downloads parent message media, fixed str(None) bug
  • tests/gateway/test_feishu.py: Updated all mocks and assertions to match new _fetch_reply_context API

How to Test

  1. Send an image message in Feishu (no text, just image)
  2. Reply to that image message with any text
  3. Verify the agent receives the original image in the reply context (not [Replying to: "None"])
  4. Check logs show: Downloaded N media attachment(s) from reply-to message

Checklist

Code

Documentation & Housekeeping

Screenshots / Logs

Before fix:

2026-05-21 10:46:21,487 INFO gateway.platforms.feishu: [Feishu] Reply context: message_id=om_x100b6fd351011080c2959095c8c8244 parent_id=om_x100b6fd21f2670b4c10f098ad1a8987 ... text='None'

After fix:

2026-05-21 12:15:30,123 INFO gateway.platforms.feishu: [Feishu] Reply context: message_id=om_xxx parent_id=om_yyy ... text='(image)' media=1
2026-05-21 12:15:30,125 INFO gateway.platforms.feishu: [Feishu] Downloaded 1 media attachment(s) from reply-to message om_yyy

PR Links

## Problem

When a user replies to an image-only message (no text), the agent sees
`[Replying to: "None"]` and cannot see the original image.

Root cause chain:
1. `_fetch_message_text()` calls `_extract_text_from_raw_content()`
2. For image messages, `text_content=""` and `metadata=None`
3. `str(None).strip()` returns `"None"` (the string), not Python `None`
4. `"None" or None` evaluates to `"None"` (truthy string)
5. The string `"None"` is cached and returned as reply_to_text
6. Media from the parent message is never fetched

## Fix

- Rename `_fetch_message_text` → `_fetch_reply_context`
- Return `(text, media_urls, media_types)` tuple instead of just text
- Download images/files from parent message and inject into current message's media
- Fix `str(None).strip() or None` → `str(placeholder).strip() if placeholder is not None else None`
- When parent has media but no text, set `reply_to_text = "(image)"`
- Add debug logging for media download operations
- Update all tests to match new API
@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 May 21, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing fix with #13115, #24285, #26063, #27545 — all address the same Feishu reply-to-image context loss (#26037). This is the 6th PR for this fix (4 still open). Oldest: #13115.

@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 both the text coercion and missing parent-media paths. The underlying issue is still present on current main: plugins/platforms/feishu/adapter.py:4211 converts a missing placeholder to the literal "None", while :3259 calls a text-only parent lookup.

Problems

  • This patch modifies gateway/platforms/feishu.py, but Feishu now lives at plugins/platforms/feishu/adapter.py after 552adbe0827c32df8ed9bb19e908c26eff43add7; GitHub reports this PR as conflicting. Current main also uses _run_blocking for SDK calls at adapter.py:4170.
  • The changed tests only update mocks and text assertions. They do not exercise an image-only parent, parent-resource download, or the emitted event's media_urls/media_types.

Suggested changes

  • Port the logic to the bundled plugin, reusing _download_feishu_message_resources at plugins/platforms/feishu/adapter.py:3771.
  • Add an end-to-end adapter-unit regression test for an image-only reply parent and assert no "None" reply text plus attached parent media.

Automated hermes-sweeper review.

return_value={"user_id": "ou_user", "user_name": "张三", "user_id_alt": None}
)
adapter._fetch_message_text = AsyncMock(return_value="父消息内容")
adapter._fetch_reply_context = AsyncMock(return_value=("父消息内容", [], []))

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 separate regression case for the actual new contract: make the reply-context mock return a parent image path/type and assert the dispatched event contains those values and never exposes reply_to_text == "None". This updated test still exercises only the text-only tuple.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 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-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.

3 participants