Skip to content

fix(feishu): stop using card action callback token as synthetic message_id - #43609

Open
Lazymonter wants to merge 1 commit into
NousResearch:mainfrom
Lazymonter:fix/feishu-card-action-reply-anchor
Open

Lazymonter wants to merge 1 commit into
NousResearch:mainfrom
Lazymonter:fix/feishu-card-action-reply-anchor

Conversation

@Lazymonter

@Lazymonter Lazymonter commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Problem

When an interactive card button is clicked, _handle_card_action_event builds a synthetic COMMAND event. Its callback token is a short-lived card-update credential, not a Feishu IM message ID.

Passing that token (or the previous random UUID fallback) as event.message_id makes _reply_anchor_for_event() use it as reply_to. Feishu rejects the response with error 99992354 (invalid message ID), so the result of a card-button command never reaches the chat.

Fix

Set message_id=None on the synthetic event:

  • the response is delivered as a regular message instead of an invalid reply;
  • the processing-reaction path skips cleanly because reactions require a real message ID;
  • deduplication is unchanged because card actions are already deduplicated by callback token before the synthetic event is built.

Testing

  • Updated the non-approval card-action test to use a realistic callback token and assert that the synthetic event has no message ID.
  • scripts/run_tests.sh tests/gateway/test_feishu_approval_buttons.py -q — 38 passed.
  • Ruff and git diff --check pass.
  • Verified on a live Feishu bot: before this change, card-button responses failed with 99992354; after it, responses are delivered as new messages.

@liuhao1024

Copy link
Copy Markdown
Contributor

Verification

Reviewed the Feishu card action callback token fix. The change sets message_id=None instead of using the callback token as the synthetic event's message_id.

Why this is correct: The callback token is a short-lived delivery token, not a Feishu message ID. When used as message_id, the subsequent command response attempts reply_to with this token, which Feishu rejects with error 99992354 — the response never reaches the chat. Setting message_id=None sends the response as a regular message instead. Dedup is already handled by _is_card_action_duplicate above, so no dedup regression.

Test coverage: The new test test_synthetic_command_carries_no_message_id verifies that event.message_id is None after routing a card action event.

Clean fix, no issues found.

@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 Jun 10, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Verification comment from automated review

Reviewed the diff. The fix is correct: the Feishu callback token is a short-lived delivery token, not a message ID. Using it as message_id causes the response's reply_to to reference an ID Feishu rejects (error 99992354), silently dropping the response.

Setting message_id=None is the right approach — it sends the response as a regular message. The comment clearly explains why, and the test test_synthetic_command_carries_no_message_id verifies the behavior.

The dedup concern is handled separately by _is_card_action_duplicate, as noted in the comment. No issues found.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for isolating the callback-token issue. The defect still exists on current main: plugins/platforms/feishu/adapter.py:3009 assigns the card callback token (or a UUID) to the synthetic event's message_id, while gateway/platforms/base.py:107 and gateway/platforms/base.py:5028-5032 use that value as the outgoing reply anchor.

Problems

  • The PR edits the former gateway/platforms/feishu.py path, but Feishu was migrated to plugins/platforms/feishu/adapter.py by 560010547. GitHub currently reports this PR as conflicting, so the patch cannot apply unchanged.

Suggested changes

  • Salvage the message_id=None assignment at plugins/platforms/feishu/adapter.py:3009 and retain the focused assertion in tests/gateway/test_feishu_approval_buttons.py.
  • The existing token deduplication remains independent at plugins/platforms/feishu/adapter.py:2969, and processing reactions already skip falsy IDs at plugins/platforms/feishu/adapter.py:3147-3150.

Automated hermes-sweeper review.

@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 14, 2026
@Lazymonter
Lazymonter force-pushed the fix/feishu-card-action-reply-anchor branch from ae1847a to b75e303 Compare July 15, 2026 18:19
wlknight pushed a commit to wlknight/hermes-agent that referenced this pull request Aug 9, 2026
When a user clicks a button on a Feishu interactive card,
_handle_card_action_event() builds a synthetic MessageEvent using the
card action callback token (c-xxx) as message_id. When the agent
replies, _reply_anchor_for_event() returns this token as reply_to,
but the Feishu ReplyMessage API requires open_message_id (om_xxx) —
the ID of the original card message. The reply fails with:

  [99992354] Invalid ids: [c-xxx]

The P2CardActionTriggerData.CallBackContext already provides
open_message_id alongside open_chat_id, but the adapter was only
extracting open_chat_id and ignoring open_message_id.

Fix: extract open_message_id from context and use it as message_id
(preferred), with token as fallback (still valid for dedup) and
uuid4 as last resort.

Production-verified: running this 2-line fix in a Docker deployment
for 7+ weeks. Card button replies that previously failed with 99992354
now deliver successfully as threaded replies to the original card.

Related: NousResearch#6422 (stale, targets pre-migration path), NousResearch#43609 (alternative
approach using message_id=None which loses reply threading)
@DavidMetcalfe

Copy link
Copy Markdown
Contributor

@Lazymonter Thanks for this — the message_id=None change correctly stops the token from being used as a reply anchor, and your live verification (99992354 → delivered) is solid.

One heads-up: issue #7200 asks for the processing-status reaction to appear on card clicks, and setting message_id=None makes on_processing_start skip the reaction entirely — so this fix resolves the reply delivery but leaves the reaction feedback unhandled. #85864 takes the other direction: the card callback payload already carries the original card message ID in event.context.open_message_id (same CallBackContext the adapter reads open_chat_id from), so the synthetic event keeps a real message ID and both the reaction and the reply anchor work.

Both approaches fix the dropped-reply bug; #85864 additionally preserves the reaction feedback #7200 expects. Happy to coordinate if you want to fold the open_message_id approach into this PR instead.

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.

5 participants