fix(feishu): route topic sends via reply API; remove invalid thread_id receive_id - #59444
Conversation
Competing with open #33584 and #37322 for the same Feishu |
Thanks for the triage note. To help a maintainer pick the canonical fix, here's why this PR necessarily touches The anchor has no source without the routing-layer changesEvery existing PR in this cluster removes the invalid
Traced end-to-end with #33584 + #55067 merged (without this PR): async-delegation completion → Why the delegate/async-delegation/run.py changes are necessary, not bundledThe maintainer note flags that this PR "also bundles delegate/async-delegation send-path changes." They're not a separate concern — they're the other half of the same fix:
The minimal-variant questionA truly minimal variant (only Feishu inbound How this relates to #37322's approach#37322 resolves the anchor at adapter-send time by querying Happy to narrow scope or adjust if a maintainer prefers a different split — but the routing-layer changes are what make the fix actually land replies in the topic, not just suppress the error. |
22414b0 to
587379d
Compare
|
Thanks for tracing the synthetic-send path end to end. The premise remains present on current The propagation changes address verified gaps: inbound Feishu currently omits No blocking defect was identified in the submitted diff. GitHub reports the PR as mergeable; the patch is a high-salvage candidate rather than stale work. This is an automated hermes-sweeper review. |
…d receive_id Async-delegation completions and terminal background notifications re-enter the originating session as synthetic events with no om_ reply anchor. The Feishu adapter's fallback branch then sent these via receive_id_type=thread_id on the create-message API, which the Feishu server rejects with [99992402] field validation failed — thread_id is not a valid receive_id_type (only open_id/union_id/user_id/email/chat_id are). A message can only land in a topic through the reply API (reply_in_thread=true) against a real om_ id. The broken branch was added in NousResearch#13077 (ff14666) and its regression test was a pure mock that only asserted the request was shaped as thread_id, never exercising the real API — a green-mock-hides-integration-bug case. Root fix — anchor everything on a stable om_ id threaded end-to-end: - Feishu inbound: populate source.message_id with the topic root (om_), so it flows into _SESSION_MESSAGE_ID and is captured by background watchers. - delegate_task(background=true): capture message_id before detaching onto the daemon worker thread (mirrors session_key capture) and carry it onto the completion event. - terminal text notifications: pass reply_to=message_id for thread routing. - _inject_watch_notification: fall back to the persisted session origin's message_id when the event lacks one. - Feishu adapter: remove the illegal receive_id_type=thread_id create branch; fall back to a top-level chat create with a warning when no anchor is available (strictly better than a hard send failure, and unreachable in normal operation once the routing layer populates anchors). Tests: - Replace the test that asserted receive_id_type=thread_id (it was freezing the bug) with reply-API-contract assertions + a no-anchor top-level fallback case. - Add Feishu inbound source.message_id tests (topic root + seed message). - Add async-delegation message_id propagation tests (single, batch, default).
587379d to
b1efd65
Compare
What does this PR do?
Fixes a Feishu send failure where the agent's reply after an async-delegation completion (and other background-notification synthetic events) is rejected by the Feishu API with
[99992402] field validation failed, because the adapter emits an invalidreceive_id_type=thread_idon the create-message API.Root cause. A message can only land in a Feishu topic via the reply API (
reply_in_thread=true) against a realom_message id. There is no "send by thread_id" path — the create-message API'sreceive_id_typeaccepts onlyopen_id/union_id/user_id/email/chat_id, notthread_id(per the create-message docs and message field docs, which definethread_idas a topic identifier distinct frommessage_id'som_prefix).The broken branch was added in #13077 (
ff14666cd) to route "reply→create fallback" messages into a topic. Its regression test (tests/gateway/test_stream_consumer_thread_routing.py::TestFeishuFallbackThreadRouting::test_create_uses_thread_id_when_available) was a pure mock that only asserted the request was shaped asreceive_id_type=thread_idand the client returnedsuccess()— it never exercised the real Feishu API, so the server rejection never surfaced. This is a green-unit-mock-hides-integration-bug case.When it triggers. Real inbound messages route fine — they carry
event.reply_to_message_id(the topic rootom_), so the adapter uses the reply API. The broken branch is only reached when a synthetic / resumed send has no reply anchor:delegate_task(background=true)completions — the async-delegation event carriessession_keybut nomessage_id.terminal(background=True, notify_on_complete=True)agent re-entry —watcher_message_idwas sourced fromsource.message_id, which Feishu never populated.terminaltext-only notifications andwatch_patternevents in a topic.All of these hit the invalid
thread_idcreate path and fail with[99992402].Why this approach. The fix keeps the existing "reply API lands the message in a topic" contract and makes the routing layer guarantee a stable
om_anchor end-to-end, so every path that needs to send into a topic reaches the reply API with a valid id. The adapter's illegal branch is removed and replaced with a defensive top-level fallback (strictly better than a hard send failure), which stays unreached in normal operation once anchors are threaded through.Related Issue
No existing issue found. Searched open/closed issues and PRs for
thread_id receive_id,field validation failed,feishu topic send,async delegation send.Fixes #
Type of Change
Changes Made
End-to-end anchor threading so a stable
om_reply id reaches every topic send, plus removal of the invalidreceive_id_type=thread_idcreate branch.Routing layer — populate the anchor and carry it to synthetic events:
plugins/platforms/feishu/adapter.py(inbound): extractroot_id, setthread_reply_anchor = root_id or message_id, and pass it asmessage_id=tobuild_source. This populatessource.message_id(previously alwaysNonefor Feishu), which flows into_SESSION_MESSAGE_IDand is captured by background watchers at spawn time.tools/approval.py: addget_current_session_message_id()helper mirroringget_current_session_key(), readingHERMES_SESSION_MESSAGE_ID.tools/delegate_tool.py: capture_message_idbefore detaching onto the daemon worker thread (mirrors_session_keycapture) and pass it todispatch_async_delegation_batch(message_id=...).tools/async_delegation.py: bothdispatch_async_delegationanddispatch_async_delegation_batchacceptmessage_id: str = "", store it on the record, and carry it onto the completion event ("message_id": ...).gateway/run.py(terminal text notifications): passreply_to=message_idon the twoadapter.send()call sites (completion + running-update) so topic-capable platforms route via the reply API.gateway/run.py(_inject_watch_notification): fall back tosource.message_id(persisted session origin) when the event lacks an explicitmessage_id— covers in-flight background processes dispatched before the anchor was captured.Adapter — remove the invalid branch:
plugins/platforms/feishu/adapter.py(_send_raw_message): delete thereceive_id_type="thread_id"create branch. When a threaded send reaches the no-anchor point anyway, fall back to a top-levelchat_idcreate with alogger.warning(thread context lost, but no hard failure). The_feishu_send_with_retry230011/231003 topic guard is untouched (preserves PR fix(gateway): stream consumer first message drops thread context #13077's intentional fail-closed for revoked-root cases).How to Test
Unit tests (hermetic):
The old test that froze the bug is replaced.
test_create_uses_thread_id_when_available(assertedreceive_id_type == "thread_id") is replaced by:test_thread_send_with_anchor_uses_reply_api— reply API +reply_in_thread=Truetest_thread_send_with_metadata_reply_to_uses_reply_api— metadata-anchor pathtest_thread_send_without_anchor_falls_back_to_chat_create— assertsreceive_id_type != "thread_id"and== "chat_id"New tests added:
tests/gateway/test_feishu.py:test_inbound_thread_message_populates_source_message_id_anchor(topic root →source.message_id),test_inbound_thread_seed_message_populates_source_message_id_self(seed message → self as root).tests/tools/test_async_delegation.py:test_completion_event_carries_message_id_single,test_completion_event_carries_message_id_batch,test_completion_event_message_id_defaults_empty(invariant: key always present, empty when no anchor).End-to-end reproduction (the original failure scenario):
delegate_task(background=true)).Before the fix, the gateway log showed:
After the fix: no
[99992402]; the reply API is used and the message appears in the topic.Cross-platform regression (other IM platforms unaffected):
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/A (no user-facing config/schema change; inline docstrings added)cli-config.yaml.exampleif I added/changed config keys — or N/A (no config keys added)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A (no architecture/workflow change)scripts/check-windows-footguns.py --diffreports 0 issues)delegate_tasksignature unchanged at the model-facing level)Screenshots / Logs
Before (async-delegation completion reply fails — IDs redacted):
After (reply routes into the topic via the reply API — no
[99992402]).