Skip to content

fix(feishu): use real card message ID for synthetic card-action events - #85864

Open
DavidMetcalfe wants to merge 4 commits into
NousResearch:mainfrom
DavidMetcalfe:fix/feishu-card-msg-id
Open

fix(feishu): use real card message ID for synthetic card-action events#85864
DavidMetcalfe wants to merge 4 commits into
NousResearch:mainfrom
DavidMetcalfe:fix/feishu-card-msg-id

Conversation

@DavidMetcalfe

@DavidMetcalfe DavidMetcalfe commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #7200 — Feishu card button clicks show no reaction feedback.

When a user clicks an interactive card button, _handle_card_action_event builds a synthetic COMMAND event. It used the card callback token (a short-lived card-update credential, not an IM message ID) as event.message_id. Downstream that breaks two things:

  1. Processing-status reactions never appearon_processing_start_add_reaction(event.message_id, "Typing") calls Feishu CreateMessageReaction with the token, which is not a valid message ID, so the API rejects it and the user sees no feedback (the issue's reported symptom).
  2. Agent responses are dropped_reply_anchor_for_event returns event.message_id for Feishu, so responses are sent with reply_to=<token> and Feishu rejects them with error 99992354 (invalid open_message_id).

The card callback payload already carries the original card message's real ID in event.context.open_message_id (the adapter already reads open_chat_id from the same CallBackContext). This PR uses it, falling back to the previous token/UUID behavior only when the payload omits it.

What this PR does

  • plugins/platforms/feishu/adapter.py: read context.open_message_id in _handle_card_action_event and use it as the synthetic event's message_id (card_message_id or token or str(uuid.uuid4())).
  • tests/gateway/test_feishu_approval_buttons.py: fixture now carries open_message_id; new test asserts the synthetic event uses it (RED before fix, GREEN after) and that the reply anchor and the processing-status reaction resolve from it; new test asserts token fallback when the field is absent.

Design decisions

  • Narrow scope: only the message_id assignment changes. The MessageType.COMMAND type and /card {tag} synthetic text are untouched — the issue is about the reaction ID, and fix(feishu): stop using card action callback token as synthetic message_id #43609 verified live that COMMAND-type synthetic events do reach the agent.
  • Fallback preserved: when a payload omits open_message_id, behavior is identical to today (token, then UUID), so malformed callbacks don't regress. A debug log on that path (added per review) keeps future 99992354 reports diagnosable.
  • Per-message reaction dedup: card actions on the same card share open_message_id, so the existing in-flight skip in on_processing_start (message_id in _pending_processing_reactions) now dedups duplicate Typing badges on the same card message as intended; on_processing_complete removes the badge and cache entry by message ID.

Notes

Verification

  • New test failed before the fix (AssertionError: 'c-card-update-credential' == 'om_original_card_msg'), passes after.
  • Mutation check: reverting the fix line (message_id=card_message_id or ...message_id=token or ...) fails the new test with 'c-card-update-credential' == 'om_original_card_msg'; restored → test_feishu_approval_buttons.py 15 passed.
  • Full Feishu suite: tests/gateway/test_feishu.py test_feishu_approval_buttons.py test_feishu_meeting_invite.py test_text_batching.py85 passed, 18 skipped.
  • git diff origin/main...HEAD --stat: 2 files, +86/−2.

dmetcalfe and others added 2 commits August 14, 2026 08:26
_handle_card_action_event built the synthetic COMMAND event with the card
callback token (or a random UUID) as message_id. The token is a short-lived
card-update credential, not an IM message ID, so the processing-status
reaction (_add_reaction via on_processing_start) and the reply anchor
(_reply_anchor_for_event) both failed: reactions never appeared and agent
responses were rejected by Feishu with error 99992354.

The callback payload carries the original card message's real ID in
event.context.open_message_id. Use it, falling back to the token only when
the payload omits it.

Closes NousResearch#7200

Co-authored-by: sline <151105947+0xsline@users.noreply.github.com>
@DavidMetcalfe
DavidMetcalfe force-pushed the fix/feishu-card-msg-id branch from 53427f7 to d70f6ce Compare August 14, 2026 15:26
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(feishu): use real card message ID for synthetic card-action events

The fix direction is correct: the card callback token is a short-lived card-update credential, not an IM message ID, so preferring context.open_message_id fixes the reaction/reply-anchor failures (99992354).

  1. plugins/platforms/feishu/adapter.py:3072context itself comes from getattr(event, "context", None), so a payload without context silently falls back to token or uuid (the previously broken behavior). Consider logging at debug level when falling back to the token, so future 99992354 reports are diagnosable without re-deriving the cause.
  2. Tests pin the synthetic event construction (event.message_id == "om_original_card_msg") but not the downstream effect the issue cites (_add_reaction / _reply_anchor_for_event). Asserting the reaction anchor resolves from the new id would lock the actual regression, not just the plumbing.
  3. Minor: str(getattr(context, "open_message_id", "") or "") — the str() wrapper is redundant since the default is already ""; harmless either way.

The callback token is not a valid IM message ID; when a payload omits
open_message_id the synthetic event is routed with the token, which fails
downstream (reaction + reply anchor, Feishu 99992354). Emit a debug log on
that path so future reports are diagnosable without re-deriving the cause.
Extend the open_message_id regression test to assert the reply anchor
(_reply_anchor_for_event) and the processing-status reaction
(on_processing_start -> _add_reaction) both resolve from the real card
message ID, and the token-fallback test to pin its reply anchor. The
event.message_id field alone is the plumbing; these assertions lock the
issue NousResearch#7200 symptom.
@DavidMetcalfe

Copy link
Copy Markdown
Contributor Author

@Enough1122 — addressed all three points; two accepted with clarifications and one declined.

  1. Debug logging on fallback — accepted (54d1e6a). One correction to the framing: a payload without context never reaches the fallback — chat_id resolves to "" and the guard at plugins/platforms/feishu/adapter.py:3080 drops the event before the synthetic event is built. The silent fallback that matters is context-present-but-open_message_id-absent, and that path now emits a debug log (adapter.py:3107-3111) before routing with the token.

  2. Downstream regression lock — accepted (4b85c88). test_synthetic_event_uses_open_message_id now additionally asserts _reply_anchor_for_event(event) == "om_original_card_msg" and drives on_processing_start(event) with _add_reaction mocked, asserting it receives the real card message ID. Mutation check: reverting the fix line (message_id=card_message_id or ...message_id=token or ...) fails the test with 'c-card-update-credential' == 'om_original_card_msg'; restored, the file passes 15/15. The fallback test also pins its reply anchor (tok_fallback_123).

  3. str() wrapper — passing on this one. It mirrors the identical str(getattr(..., "") or "") extraction used for token (3065), chat_id (3071), and open_id (3079) in the same function; removing it just for the new line would diverge from the local pattern for zero functional gain. Happy to revisit if you disagree.

One note for future readers: since card actions on the same card share open_message_id, the per-message processing-reaction cache (_pending_processing_reactions) now dedups as intended — a second click while a turn is in flight won't stack a duplicate Typing badge, and on_processing_complete removes the badge and cache entry by message ID.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/feishu Feishu / Lark adapter 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.

[Bug]: 接收飞书卡片的回调过程中,没有正确获取原始卡片消息的 ID 来添加表情

3 participants