From bce5dc3327cd2dae6be73da4cd5523334b0b5013 Mon Sep 17 00:00:00 2001 From: Loveca <44801706+Loveca@users.noreply.github.com> Date: Thu, 12 Nov 2026 22:09:20 +0800 Subject: [PATCH] fix(feishu): receive post attachments from the top-level files array (#8190) * fix(feishu): keep attachments sent with post messages * fix(feishu): keep user-typed file placeholders and malformed files arrays from losing attachments * test(feishu): pin mixed inline attachment behavior and consume the post file count * fix(feishu): parse post commands before adding attachment placeholders --------- Co-authored-by: Willem Jiang Co-authored-by: Totoro --- backend/app/channels/AGENTS.md | 3 + backend/app/channels/feishu.py | 68 ++++++- backend/tests/test_feishu_parser.py | 300 ++++++++++++++++++++++++++++ docs/guide/im-channels.md | 4 + 4 files changed, 373 insertions(+), 2 deletions(-) diff --git a/backend/app/channels/AGENTS.md b/backend/app/channels/AGENTS.md index 13917f0445..4a661f33d1 100644 --- a/backend/app/channels/AGENTS.md +++ b/backend/app/channels/AGENTS.md @@ -60,6 +60,9 @@ make test-blocking-io # channels share the Gate `/start `) is handled before `allowed_users` on purpose, so that a new user can bind. The code is the secret: show it only in the browser that requested it and never echo it to the chat. +- **Feishu post commands use the caption.** Add placeholders for top-level post + attachments after command detection. For commands that enter the chat path, + `receive_file` adds their placeholders after the manager parses the arguments. - **One active owner per external identity.** A partial unique index in the database enforces this. Application code alone cannot, because two workers can bind at the same time. diff --git a/backend/app/channels/feishu.py b/backend/app/channels/feishu.py index d6216c8b6e..2503b6da2f 100644 --- a/backend/app/channels/feishu.py +++ b/backend/app/channels/feishu.py @@ -55,6 +55,9 @@ PENDING_CLARIFICATION_TTL_SECONDS = 30 * 60 FEISHU_STOP_TIMEOUT_SECONDS = 5 FEISHU_WS_CLOSE_TIMEOUT_SECONDS = 2 _PENDING_CARD_METADATA_KEY = "_feishu_pending_card_id" +# Number of attachments from a post's top-level ``files`` array. +_POST_FILES_COUNT_METADATA_KEY = "_feishu_post_files_count" +_DEFER_POST_FILE_PLACEHOLDERS_METADATA_KEY = "_feishu_defer_post_file_placeholders" # Channel-store deadlines. ``_on_message`` runs on lark-oapi's private event loop, # which also carries the SDK's websocket reads, pongs and its 120s ping loop: # every second the bridge blocks that loop is a second the connection cannot @@ -107,6 +110,7 @@ class _ParsedMessage: chat_type: str | None text: str files: list[dict[str, Any]] + post_file_count: int = 0 msg_type: InboundMessageType = InboundMessageType.CHAT @@ -242,6 +246,12 @@ def _is_feishu_command(text: str) -> bool: return is_known_channel_command(text) +def _append_post_file_placeholders(text: str, count: int) -> str: + paragraphs = [text] if text else [] + paragraphs.extend(["[file]"] * count) + return "\n\n".join(paragraphs) + + class FeishuChannel(Channel): """Feishu/Lark IM channel using the ``lark-oapi`` WebSocket client. @@ -809,6 +819,10 @@ class FeishuChannel(Channel): logger.warning("[Feishu] received message with no files: %s", msg) return msg text = msg.text + post_file_count = msg.metadata.pop(_POST_FILES_COUNT_METADATA_KEY, 0) + defer_post_file_placeholders = msg.metadata.pop(_DEFER_POST_FILE_PLACEHOLDERS_METADATA_KEY, False) + if not isinstance(post_file_count, int) or post_file_count < 0: + post_file_count = 0 search_from = 0 def replace_next_placeholder(placeholder: str, replacement: str) -> None: @@ -819,13 +833,36 @@ class FeishuChannel(Channel): text = f"{text[:idx]}{replacement}{text[idx + len(placeholder) :]}" search_from = idx + len(replacement) + downloaded: list[tuple[str, str]] = [] for file in files: if file.get("image_key"): virtual_path = await self._receive_single_file(msg.thread_ts, file["image_key"], "image", thread_id, user_id=user_id) - replace_next_placeholder("[image]", virtual_path) + downloaded.append(("[image]", virtual_path)) elif file.get("file_key"): virtual_path = await self._receive_single_file(msg.thread_ts, file["file_key"], "file", thread_id, user_id=user_id) - replace_next_placeholder("[file]", virtual_path) + downloaded.append(("[file]", virtual_path)) + + # Top-level post attachments append their ``[file]`` placeholders + # after the parsed body, so their slots are the final + # ``post_file_count`` ones. Fill them from the end first: a literal + # ``[file]`` typed by the user must not consume a top-level + # attachment's path. Inline rich-text attachments keep the historical + # left-to-right matching below; that path is deliberately unchanged. + post_file_count = min(post_file_count, len(downloaded)) + if defer_post_file_placeholders is True: + # A command such as /goal has now entered the chat path. Add the + # generated slots to the chat text after its arguments were parsed. + text = _append_post_file_placeholders(text, post_file_count) + suffix_end = len(text) + for placeholder, virtual_path in reversed(downloaded[len(downloaded) - post_file_count :] if post_file_count else []): + idx = text.rfind(placeholder, 0, suffix_end) + if idx < 0: + continue + text = f"{text[:idx]}{virtual_path}{text[idx + len(placeholder) :]}" + suffix_end = idx + + for placeholder, virtual_path in downloaded[: len(downloaded) - post_file_count]: + replace_next_placeholder(placeholder, virtual_path) msg.text = text # Clear the descriptors so the generic URL-based _ingest_inbound_files # pass does not try to re-fetch files that were downloaded by key. @@ -1750,6 +1787,10 @@ class FeishuChannel(Channel): } if source_preview: metadata[SOURCE_PREVIEW_METADATA_KEY] = source_preview + if parsed.post_file_count: + metadata[_POST_FILES_COUNT_METADATA_KEY] = parsed.post_file_count + if parsed.msg_type == InboundMessageType.COMMAND: + metadata[_DEFER_POST_FILE_PLACEHOLDERS_METADATA_KEY] = True if routing.clarification_card_id: metadata[_PENDING_CARD_METADATA_KEY] = routing.clarification_card_id @@ -2088,6 +2129,26 @@ class FeishuChannel(Channel): text = "" text = text.strip() + # Feishu desktop attaches files to a message as a post whose rich-text + # body holds the caption and whose top-level ``files`` array holds the + # attachments. The body may be empty. Keep these attachments last so + # ``receive_file`` can match their placeholders from the end. Skip + # folders and attachments already represented by a rich-text element. + attachments = content.get("files") if isinstance(content, dict) else None + if not isinstance(attachments, list): + attachments = [] + post_file_count = 0 + for attachment in attachments: + if not isinstance(attachment, dict) or attachment.get("is_folder"): + continue + file_key = attachment.get("file_key") + if not isinstance(file_key, str) or not file_key: + continue + if any(existing.get("file_key") == file_key for existing in files_list): + continue + files_list.append({"file_key": file_key}) + post_file_count += 1 + logger.info( "[Feishu] parsed message: chat_id=%s, msg_id=%s, root_id=%s, parent_id=%s, thread_id=%s, chat_type=%s, sender=%s, text_len=%d", chat_id, @@ -2113,6 +2174,7 @@ class FeishuChannel(Channel): chat_type=chat_type, text=text, files=files_list, + post_file_count=post_file_count, ) def _on_message(self, event) -> None: @@ -2155,6 +2217,8 @@ class FeishuChannel(Channel): if _is_feishu_command(command_text): parsed.msg_type = InboundMessageType.COMMAND parsed.text = command_text + elif parsed.post_file_count: + parsed.text = _append_post_file_placeholders(text, parsed.post_file_count) # topic_id determines which LangGraph thread the message maps to. # P2P chats: topic_id=None so all messages share one thread (like Telegram DMs). diff --git a/backend/tests/test_feishu_parser.py b/backend/tests/test_feishu_parser.py index 2b6188eb63..5adad39e05 100644 --- a/backend/tests/test_feishu_parser.py +++ b/backend/tests/test_feishu_parser.py @@ -19,6 +19,7 @@ from app.channels.message_bus import ( PENDING_CLARIFICATION_METADATA_KEY, RESOLVED_FROM_PENDING_CLARIFICATION_METADATA_KEY, InboundMessage, + InboundMessageType, MessageBus, OutboundMessage, ) @@ -1220,6 +1221,305 @@ def test_feishu_store_bridge_refuses_to_block_the_gateway_loop_itself(caplog): _run(go()) +def _make_post_event(content: dict, *, message_id: str = "msg_post"): + event = MagicMock() + event.event.message.chat_id = "chat_1" + event.event.message.message_id = message_id + event.event.message.root_id = None + event.event.message.parent_id = None + event.event.message.thread_id = None + event.event.sender.sender_id.open_id = "user_1" + event.event.message.content = json.dumps(content) + return event + + +def test_feishu_post_with_text_and_top_level_files_extracts_attachments(): + """Feishu desktop attaches files to a post message in a top-level ``files`` array.""" + channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test"}) + event = _make_post_event( + { + "title": "", + "content": [[{"tag": "text", "text": "帮我核对这两张发票和采购订单，看看有没有差异。", "style": []}]], + "content_v2": [[{"tag": "text", "text": "帮我核对这两张发票和采购订单，看看有没有差异。", "style": []}]], + "files": [ + {"file_key": "file_inv_a", "file_name": "INV-A-20260815.csv", "is_folder": False}, + {"file_key": "file_inv_b", "file_name": "INV-B-20260903.csv", "is_folder": False}, + {"file_key": "file_po", "file_name": "PO-2026-08.csv", "is_folder": False}, + ], + } + ) + + with pytest.MonkeyPatch.context() as m: + mock_make_inbound = MagicMock() + m.setattr(channel, "_make_inbound", mock_make_inbound) + channel._on_message(event) + + mock_make_inbound.assert_called_once() + assert mock_make_inbound.call_args[1]["text"] == "帮我核对这两张发票和采购订单，看看有没有差异。\n\n[file]\n\n[file]\n\n[file]" + assert mock_make_inbound.call_args[1]["files"] == [{"file_key": "file_inv_a"}, {"file_key": "file_inv_b"}, {"file_key": "file_po"}] + + +@pytest.mark.parametrize("code", ["bind-code", None]) +def test_feishu_post_connect_reads_only_the_caption(monkeypatch, code): + channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test", "connection_repo": object()}) + channel._main_loop = MagicMock() + bind = MagicMock() + submit = MagicMock(return_value=True) + monkeypatch.setattr(channel, "_bind_connection_from_connect_code", bind) + monkeypatch.setattr(channel, "_submit_threadsafe_coroutine", submit) + monkeypatch.setattr(channel, "_schedule_prepare_inbound", MagicMock()) + caption = f"@_user_1 /connect {code}" if code else "@_user_1 /connect" + event = _make_post_event({"content": [[{"tag": "text", "text": caption}]], "files": [{"file_key": "file_a"}]}) + + channel._on_message(event) + + if code: + bind.assert_called_once() + assert bind.call_args.kwargs["code"] == code + submit.assert_called_once() + else: + bind.assert_not_called() + submit.assert_not_called() + + +@pytest.mark.asyncio +@pytest.mark.parametrize(("argument", "operation"), [("clear", "delete"), ("compare [file]", "put")]) +async def test_feishu_post_goal_parses_caption_before_loading_files(tmp_path, monkeypatch, argument, operation): + from app.channels.manager import ChannelManager + + bus = MessageBus() + channel = FeishuChannel(bus, {"app_id": "test", "app_secret": "test"}) + schedule = MagicMock() + monkeypatch.setattr(channel, "_schedule_prepare_inbound", schedule) + event = _make_post_event( + { + "content": [[{"tag": "at", "text": "@_user_1"}, {"tag": "text", "text": f"/goal {argument}"}]], + "files": [{"file_key": "file_a"}, {"file_key": "file_b"}], + } + ) + channel._on_message(event) + schedule.assert_called_once() + inbound = schedule.call_args.args[1] + assert inbound.msg_type == InboundMessageType.COMMAND + + manager = ChannelManager(bus, JsonChannelStore(tmp_path / "bindings.json")) + goal_request = AsyncMock() + monkeypatch.setattr(manager, "_goal_request", goal_request) + monkeypatch.setattr(manager, "_lookup_thread_id", AsyncMock(return_value="thread_1")) + download = AsyncMock(side_effect=["/mnt/user-data/uploads/A.csv", "/mnt/user-data/uploads/B.csv"]) + monkeypatch.setattr(channel, "_receive_single_file", download) + + async def handle_chat(msg, **kwargs): + assert msg.text == argument + result = await channel.receive_file(msg, "thread_1") + assert result.text == f"{argument}\n\n/mnt/user-data/uploads/A.csv\n\n/mnt/user-data/uploads/B.csv" + + chat = AsyncMock(side_effect=handle_chat) + monkeypatch.setattr(manager, "_handle_chat", chat) + await manager._handle_command(inbound) + + goal_request.assert_awaited_once() + assert goal_request.await_args.args[:2] == (operation, "thread_1") + if operation == "put": + assert goal_request.await_args.kwargs["json"] == {"objective": argument} + chat.assert_awaited_once() + assert download.await_count == 2 + else: + chat.assert_not_awaited() + download.assert_not_awaited() + + +def test_feishu_files_only_post_is_not_treated_as_empty(): + """A files-only post has an empty rich-text body; the attachments still make it non-empty.""" + channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test"}) + event = _make_post_event( + { + "title": "", + "content": [[]], + "content_v2": [[]], + "files": [ + {"file_key": "file_a", "file_name": "A.csv", "is_folder": False}, + {"file_key": "file_b", "file_name": "B.csv", "is_folder": False}, + ], + } + ) + + with pytest.MonkeyPatch.context() as m: + mock_make_inbound = MagicMock() + m.setattr(channel, "_make_inbound", mock_make_inbound) + channel._on_message(event) + + mock_make_inbound.assert_called_once() + assert mock_make_inbound.call_args[1]["text"] == "[file]\n\n[file]" + assert mock_make_inbound.call_args[1]["files"] == [{"file_key": "file_a"}, {"file_key": "file_b"}] + + +def test_feishu_post_skips_folders_and_duplicate_attachments(): + channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test"}) + event = _make_post_event( + { + "title": "", + "content": [[{"tag": "text", "text": "见附件"}, {"tag": "file", "file_key": "file_a"}]], + "files": [ + {"file_key": "file_a", "file_name": "A.csv", "is_folder": False}, + {"file_key": "folder_x", "file_name": "资料", "is_folder": True}, + ], + } + ) + + with pytest.MonkeyPatch.context() as m: + mock_make_inbound = MagicMock() + m.setattr(channel, "_make_inbound", mock_make_inbound) + channel._on_message(event) + + mock_make_inbound.assert_called_once() + assert mock_make_inbound.call_args[1]["text"] == "见附件 [file]" + assert mock_make_inbound.call_args[1]["files"] == [{"file_key": "file_a"}] + + +def test_feishu_post_merges_inline_and_top_level_files_in_order(): + """Inline rich-text files come first; top-level files append without re-adding duplicates.""" + channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test"}) + event = _make_post_event( + { + "title": "", + "content": [[{"tag": "text", "text": "见附件"}, {"tag": "file", "file_key": "file_inline"}]], + "files": [ + {"file_key": "file_inline", "file_name": "inline.csv", "is_folder": False}, + {"file_key": "file_top1", "file_name": "top1.csv", "is_folder": False}, + {"file_key": "file_top2", "file_name": "top2.csv", "is_folder": False}, + ], + } + ) + + with pytest.MonkeyPatch.context() as m: + mock_make_inbound = MagicMock() + m.setattr(channel, "_make_inbound", mock_make_inbound) + channel._on_message(event) + + mock_make_inbound.assert_called_once() + assert mock_make_inbound.call_args[1]["text"] == "见附件 [file]\n\n[file]\n\n[file]" + assert mock_make_inbound.call_args[1]["files"] == [ + {"file_key": "file_inline"}, + {"file_key": "file_top1"}, + {"file_key": "file_top2"}, + ] + + +def test_feishu_post_user_written_file_placeholder_is_not_replaced(): + """A literal ``[file]`` typed by the user must not consume an attachment's path.""" + channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test"}) + event = _make_post_event( + { + "title": "", + "content": [[{"tag": "text", "text": "请按 [file] 格式整理", "style": []}]], + "files": [ + {"file_key": "file_a", "file_name": "A.csv", "is_folder": False}, + {"file_key": "file_b", "file_name": "B.csv", "is_folder": False}, + ], + } + ) + + with pytest.MonkeyPatch.context() as m: + mock_make_inbound = MagicMock() + m.setattr(channel, "_make_inbound", mock_make_inbound) + channel._on_message(event) + + kwargs = mock_make_inbound.call_args[1] + assert kwargs["text"] == "请按 [file] 格式整理\n\n[file]\n\n[file]" + assert kwargs["metadata"][feishu_module._POST_FILES_COUNT_METADATA_KEY] == 2 + + async def go(): + msg = InboundMessage( + channel_name="feishu", + chat_id="chat_1", + user_id="user_1", + text=kwargs["text"], + thread_ts="msg_post", + files=kwargs["files"], + metadata=kwargs["metadata"], + ) + channel._receive_single_file = AsyncMock(side_effect=["/mnt/user-data/uploads/A.csv", "/mnt/user-data/uploads/B.csv"]) + result = await channel.receive_file(msg, "thread_1") + assert result.text == "请按 [file] 格式整理\n\n/mnt/user-data/uploads/A.csv\n\n/mnt/user-data/uploads/B.csv" + + _run(go()) + + +def test_feishu_post_mixed_inline_and_top_level_keeps_appended_slot_last(): + """Pin the mixed inline + top-level behavior: only the appended slot is protected. + + Inline rich-text attachments still match placeholders left to right, exactly as + on ``main``, so a user-typed ``[file]`` ahead of an inline attachment is consumed + first. This test documents that pre-existing limitation; the fix here only + guarantees the appended top-level slot (the last one) is never displaced. + """ + channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test"}) + event = _make_post_event( + { + "title": "", + "content": [ + [{"tag": "text", "text": "见 [file] 整理", "style": []}], + [{"tag": "file", "file_key": "file_inline"}], + ], + "files": [{"file_key": "file_top", "file_name": "top.csv", "is_folder": False}], + } + ) + + with pytest.MonkeyPatch.context() as m: + mock_make_inbound = MagicMock() + m.setattr(channel, "_make_inbound", mock_make_inbound) + channel._on_message(event) + + kwargs = mock_make_inbound.call_args[1] + assert kwargs["text"] == "见 [file] 整理\n\n[file]\n\n[file]" + assert kwargs["files"] == [{"file_key": "file_inline"}, {"file_key": "file_top"}] + assert kwargs["metadata"][feishu_module._POST_FILES_COUNT_METADATA_KEY] == 1 + + async def go(): + msg = InboundMessage( + channel_name="feishu", + chat_id="chat_1", + user_id="user_1", + text=kwargs["text"], + thread_ts="msg_post", + files=kwargs["files"], + metadata=kwargs["metadata"], + ) + channel._receive_single_file = AsyncMock(side_effect=["/mnt/user-data/uploads/inline.csv", "/mnt/user-data/uploads/top.csv"]) + result = await channel.receive_file(msg, "thread_1") + # Pre-existing inline behavior: the user-typed literal is consumed first and + # the inline slot keeps its ``[file]``; the appended top-level slot is last. + assert result.text == "见 /mnt/user-data/uploads/inline.csv 整理\n\n[file]\n\n/mnt/user-data/uploads/top.csv" + # The count is consumed, so it does not ride along on outbound metadata. + assert feishu_module._POST_FILES_COUNT_METADATA_KEY not in msg.metadata + + _run(go()) + + +def test_feishu_post_ignores_non_list_files_field(): + """A malformed top-level ``files`` value must not take the whole message down.""" + channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test"}) + event = _make_post_event( + { + "title": "", + "content": [[{"tag": "text", "text": "先看文字", "style": []}]], + "files": 3, + } + ) + + with pytest.MonkeyPatch.context() as m: + mock_make_inbound = MagicMock() + m.setattr(channel, "_make_inbound", mock_make_inbound) + channel._on_message(event) + + mock_make_inbound.assert_called_once() + kwargs = mock_make_inbound.call_args[1] + assert kwargs["text"] == "先看文字" + assert kwargs["files"] == [] + assert feishu_module._POST_FILES_COUNT_METADATA_KEY not in kwargs["metadata"] + + def _make_text_event( text: str, *, diff --git a/docs/guide/im-channels.md b/docs/guide/im-channels.md index c3763ed0e4..da2892872f 100644 --- a/docs/guide/im-channels.md +++ b/docs/guide/im-channels.md @@ -55,6 +55,10 @@ become visible only after the complete copy is published. Feishu and DingTalk attachment transfers return standalone temporary sandboxes to the pool after synchronization, without interrupting an active run. +Feishu desktop posts can include a caption and files. Commands use the caption, +and a command that starts a chat turn, such as `/goal `, passes the +files to that turn. + **Configuration in `config.yaml`:** Discord's `channels.discord.allowed_guilds` accepts one positive numeric guild