From a9d9e63d4b9c9bdae801f653d5bcc6eb37c37c01 Mon Sep 17 00:00:00 2001 From: richardmilles Date: Fri, 7 Aug 2026 10:18:06 +0200 Subject: [PATCH] fix(channels): keep empty topic_id distinct from missing topic Empty topic_id no longer collapses onto the base conversation mapping. --- backend/app/channels/store.py | 7 ++++++- backend/tests/test_channels.py | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/backend/app/channels/store.py b/backend/app/channels/store.py index 81f5d61e421..12e43aefa79 100644 --- a/backend/app/channels/store.py +++ b/backend/app/channels/store.py @@ -73,7 +73,12 @@ def _save(self) -> None: @staticmethod def _key(channel_name: str, chat_id: str, topic_id: str | None = None) -> str: - if topic_id: + # Use ``is not None`` (not truthiness) so an empty ``topic_id`` stays a + # distinct key from the topic-less base mapping. ``remove()`` already + # treats any non-None topic_id as topic-specific; collapsing ``""`` here + # would let a topic write/delete clobber the base ``channel:chat`` entry + # (e.g. DingTalk group messages whose ``message_id`` is missing). + if topic_id is not None: return f"{channel_name}:{chat_id}:{topic_id}" return f"{channel_name}:{chat_id}" diff --git a/backend/tests/test_channels.py b/backend/tests/test_channels.py index e5f54dd3ecc..b982c243c6e 100644 --- a/backend/tests/test_channels.py +++ b/backend/tests/test_channels.py @@ -289,6 +289,23 @@ def test_corrupt_file_handled(self, tmp_path): store = ChannelStore(path=path) assert store.get_thread_id("x", "y") is None + def test_empty_topic_id_is_distinct_from_none(self, store): + """Empty topic_id must not collapse onto the topic-less base key. + + Channels such as DingTalk may pass ``topic_id=""`` when a group + message lacks ``message_id``. Truthy keying would overwrite/delete the + base conversation mapping instead of a topic-specific entry. + """ + store.set_thread_id("dingtalk", "conv", "base-thread", topic_id=None) + store.set_thread_id("dingtalk", "conv", "empty-topic-thread", topic_id="") + + assert store.get_thread_id("dingtalk", "conv") == "base-thread" + assert store.get_thread_id("dingtalk", "conv", topic_id="") == "empty-topic-thread" + + assert store.remove("dingtalk", "conv", topic_id="") is True + assert store.get_thread_id("dingtalk", "conv", topic_id="") is None + assert store.get_thread_id("dingtalk", "conv") == "base-thread" + # --------------------------------------------------------------------------- # Channel base class tests