fix: route feishu thread sends via reply API instead of invalid thread_id receive_id - #75940
fix: route feishu thread sends via reply API instead of invalid thread_id receive_id#75940Cassius0924 wants to merge 2 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the existing audio fallback and extending the actual reported document path; current main still has the audio-only guard at plugins/platforms/feishu/adapter.py:4711-4714.
Problems
- The retry remains inside the non-caption branch (
plugins/platforms/feishu/adapter.py:4701-4738). Captioned file/video sends take thepostpath atplugins/platforms/feishu/adapter.py:4688-4700and still have no 99992402 recovery.send_animation()always reaches that captioned document path atplugins/platforms/feishu/adapter.py:2366-2374. - The no-anchor test's
createmock always fails with 99992402 (tests/gateway/test_feishu.py:1352-1358) and the test only checks the final request (tests/gateway/test_feishu.py:1399-1401), so it does not establish that the fallback succeeds.
Suggested changes
- Share the retry across both initial payload branches and cover a captioned file/media send.
- Make the second
createcall succeed in the no-anchor fixture, then assertresult.success.
Automated hermes-sweeper review.
| if (not self._response_succeeded(message_response) | ||
| and getattr(message_response, "code", None) == 99992402 | ||
| and resolved_message_type == "audio" |
There was a problem hiding this comment.
Removing this guard fixes the non-captioned document/video path, but this retry is still entirely inside the else branch. Captioned file/video sends use the preceding post branch and continue to return 99992402 without recovery; please share the retry across both payload branches.
There was a problem hiding this comment.
Resolved — the PR was reworked to a source-level fix. The retry guard (and its per-branch limitation) is gone: _send_uploaded_file_message's caption and non-caption branches both call _feishu_send_with_retry → _send_raw_message, which no longer builds the invalid receive_id=thread_id request at all (it lists the thread and replies to the last message instead). Captioned file/video sends can no longer hit 99992402. Added test_source_fix_caption_thread_no_anchor_lists_then_replies covering the captioned post path.
| def create(self, request): | ||
| calls.append(("create", request)) | ||
| return SimpleNamespace( | ||
| success=lambda: False, |
There was a problem hiding this comment.
This fixture returns 99992402 for both creates, while the test only verifies the final request. Make the chat-id create succeed and assert result.success so the regression proves the fallback delivers rather than merely routes.
There was a problem hiding this comment.
Resolved — the old no-anchor fixture was removed with the retry approach. The replacement test test_source_fix_thread_no_anchor_no_last_msg_creates_chat_id makes the chat_id create succeed and asserts result.success, so the regression proves the fallback delivers (and preserves the user_id receive_id mapping).
|
Thanks for this PR — this is the exact failure I hit on a multi-profile deployment (Feishu gateway, thread_id routing, 99992402 on non-audio sends). I am totally new to coding/github but my deepseek LLM seems to have a different approach to this, so I took some courage and post it here in case it might help. Our approach: fix at the _send_raw_message level instead of the retry level Instead of extending the 99992402 retry to more message types, we changed the fallback path in _send_raw_message (the topic/thread branch that previously passed thread_id as receive_id) to: Try to anchor the reply on the last message in the thread via ListMessageRequest (container_id_type="thread", page_size=1) — the existing _fetch_last_message_in_thread helper. If an anchor is found → use the reply API with reply_in_thread=True (message lands in the topic, never falls to main chat). If no anchor → log a warning, then fall back to plain chat_id create honoring the feishu_user_id: → user_id and ou_ → open_id mapping. This fixes the problem at the source (create-with-thread_id is invalid for all message types), so captioned/video/file/audio all route correctly — no per-type retry guards needed. The thread-id fallback branch is in _send_raw_message around line 4786 in current main. Diff (against 40e0e7a, the commit we were on)
Trade-off vs. this PR Our approach fixes the routing for all message types at the source (one branch, no per-type guards), but it adds a list API call on every thread message without a reply anchor (1 extra round-trip in the fallback path only). This PR's retry approach is cheaper for the happy path (no extra call unless 99992402), at the cost of per-type guard maintenance. Happy to adjust either direction — just wanted to make sure the captioned/file branch coverage question had a concrete option. Pardon me if the reply is not as professional as others. |
a355cc9 to
043bd98
Compare
|
Thanks for this suggestion — it's the right call. I applied your source-level approach and reworked the PR: Updated PR: #75940 |
…d_id receive_id The Feishu message.create API does not accept thread_id as receive_id, so sends into a thread without a reply anchor (media/file delivery metadata carries only thread_id) failed with [99992402] field validation failed. Fix _send_raw_message to never build that invalid request: when a thread_id is present but no reply anchor exists, list the thread via ListMessageRequest and reply to the last message (reply_in_thread=true); if the thread is empty, fall back to a plain chat_id create (keeping the feishu_user_id:/ou_ receive_id mapping). This fixes all message types (file/media/audio/captioned post payloads) at the source instead of per-type retry guards.
043bd98 to
daa3b47
Compare
Problem
Sending files/media into a Feishu group thread fails with
[99992402] field validation failedwhen the send has no reply anchor. See issue for repro.Root cause
The Feishu
message.createAPI only acceptschat_id/open_id/user_idasreceive_id. When media/file delivery carries onlythread_id(no reply anchor),_send_raw_message()built a create request withreceive_id=thread_id, which the API rejects with[99992402]. The existing retry only coveredaudiomessages, so file/media sends failed outright.Fix
Never build the invalid request. In
_send_raw_message(), the thread_id fallback branch now:ListMessageRequest(container_id_type=thread,page_size=1) and replies to the last message withreply_in_thread=true(message lands in the topic)chat_idcreate, keeping thefeishu_user_id:→user_idandou_→open_idreceive_id mappingThis fixes all message types (file/media/audio/captioned post payloads) at the source — no per-type retry guards needed. Credit to @kevinjihk for this approach (comment on the earlier revision).
Verification
test_source_fix_thread_no_anchor_lists_then_replies(never calls create with thread_id),test_source_fix_thread_no_anchor_no_last_msg_creates_chat_id(empty thread falls back to chat_id with user_id mapping) andtest_source_fix_caption_thread_no_anchor_lists_then_replies(captioned post path gets the same routing)tests/gateway/test_feishu.pysuite: 79 passed.jsfile into a thread now succeeds with no error in gateway logsFixes #75939