fix(ltm): Web UI 删除对话/平台后 LTM 记忆未清理 & unique_session 下群聊记录隔离 (#8386)#8421
fix(ltm): Web UI 删除对话/平台后 LTM 记忆未清理 & unique_session 下群聊记录隔离 (#8386)#8421Ayleovelle wants to merge 2 commits into
Conversation
…Devs#8386) - Bug 1: delete_conversation 现在触发 _trigger_session_deleted 回调,清理 LTM session_chats - Bug 2: 删除平台适配器时级联删除该平台所有对话记录 - Bug 3: LTM 使用 group-level key 存储群聊记录,不受 unique_session 影响 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request implements a cleanup mechanism for Long-Term Memory (LTM) session chats when conversations or platforms are deleted. It introduces a _group_key helper to handle group-level keys, registers a deletion callback, and adds comprehensive unit tests. The review feedback correctly identifies a bug in parsing the group ID from the unified message origin (umo) when unique_session is enabled, and provides actionable code suggestions to resolve this in both the main logic and the tests.
| async def _clear_ltm_session(umo: str) -> None: | ||
| self.ltm.session_chats.pop(umo, None) | ||
| # Also clear group-level key for unique_session scenarios | ||
| parts = umo.split(":") | ||
| if len(parts) >= 3 and parts[1] == "GroupMessage": | ||
| group_key = f"{parts[0]}:GroupMessage:{parts[2]}" | ||
| self.ltm.session_chats.pop(group_key, None) |
There was a problem hiding this comment.
在开启 unique_session 时,umo 的第三部分(即 parts[2])会被改写为 user_id%group_id 的格式。
如果直接使用 parts[2] 作为 group_key 的一部分,那么生成的 group_key 将会是 platform:GroupMessage:user_id%group_id。
然而,在 LongTermMemory._group_key 中,群聊的 key 是通过 event.get_group_id() 获取的,即 platform:GroupMessage:group_id。
这会导致 _clear_ltm_session 无法正确清理实际存储在 session_chats 中的群聊记忆(因为 key 不匹配)。
建议通过 parts[2].split("%")[-1] 提取出真正的 group_id,以确保在 unique_session 开启和关闭的情况下都能正确清理 LTM 记忆。
| async def _clear_ltm_session(umo: str) -> None: | |
| self.ltm.session_chats.pop(umo, None) | |
| # Also clear group-level key for unique_session scenarios | |
| parts = umo.split(":") | |
| if len(parts) >= 3 and parts[1] == "GroupMessage": | |
| group_key = f"{parts[0]}:GroupMessage:{parts[2]}" | |
| self.ltm.session_chats.pop(group_key, None) | |
| async def _clear_ltm_session(umo: str) -> None: | |
| self.ltm.session_chats.pop(umo, None) | |
| # Also clear group-level key for unique_session scenarios | |
| parts = umo.split(":") | |
| if len(parts) >= 3 and parts[1] == "GroupMessage": | |
| group_id = parts[2].split("%")[-1] | |
| group_key = f"{parts[0]}:GroupMessage:{group_id}" | |
| self.ltm.session_chats.pop(group_key, None) |
There was a problem hiding this comment.
已修复
现在把 parts[2] 改成了 parts[2].split("%")[-1]。当 unique_session 关闭时 parts[2] 就是纯
group_id,split("%")[-1] 结果不变;开启时 parts[2] 是 userId%groupId,split("%")[-1] 取到最后一段就是真正的 group_id。
| async def _clear_ltm_session(origin: str) -> None: | ||
| ltm.session_chats.pop(origin, None) | ||
| parts = origin.split(":") | ||
| if len(parts) >= 3 and parts[1] == "GroupMessage": | ||
| group_key = f"{parts[0]}:GroupMessage:{parts[2]}" | ||
| ltm.session_chats.pop(group_key, None) |
There was a problem hiding this comment.
更新测试中的 _clear_ltm_session 辅助函数,使其与 main.py 中修复后的逻辑保持一致,支持在 unique_session 场景下正确提取 group_id。
| async def _clear_ltm_session(origin: str) -> None: | |
| ltm.session_chats.pop(origin, None) | |
| parts = origin.split(":") | |
| if len(parts) >= 3 and parts[1] == "GroupMessage": | |
| group_key = f"{parts[0]}:GroupMessage:{parts[2]}" | |
| ltm.session_chats.pop(group_key, None) | |
| async def _clear_ltm_session(origin: str) -> None: | |
| ltm.session_chats.pop(origin, None) | |
| parts = origin.split(":") | |
| if len(parts) >= 3 and parts[1] == "GroupMessage": | |
| group_id = parts[2].split("%")[-1] | |
| group_key = f"{parts[0]}:GroupMessage:{group_id}" | |
| ltm.session_chats.pop(group_key, None) |
Addresses Gemini review: when unique_session is enabled, umo contains
user_id%group_id in the third segment. Use split("%")[-1] to get the
actual group_id for LTM cleanup.
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
AstrBot.__init__the_clear_ltm_sessioncallback derives the group-level key by splittingumoon:and assuming aplatform:GroupMessage:<group>layout; this can easily drift from_group_key()’s logic (especially underunique_sessionwhere the third segment may beuser%group), so it would be more robust to reuse the same key derivation logic (or a shared helper) instead of manual string parsing. - In
ConversationManager.delete_conversation,_trigger_session_deletedis called for theunified_msg_origineven whenconversation_iddoes not matchcurr_cid, which can cause session-level cleanups (e.g., LTM) to run without the current session actually being cleared; consider calling_trigger_session_deletedonly when the requested conversation was the active one and has been removed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `AstrBot.__init__` the `_clear_ltm_session` callback derives the group-level key by splitting `umo` on `:` and assuming a `platform:GroupMessage:<group>` layout; this can easily drift from `_group_key()`’s logic (especially under `unique_session` where the third segment may be `user%group`), so it would be more robust to reuse the same key derivation logic (or a shared helper) instead of manual string parsing.
- In `ConversationManager.delete_conversation`, `_trigger_session_deleted` is called for the `unified_msg_origin` even when `conversation_id` does not match `curr_cid`, which can cause session-level cleanups (e.g., LTM) to run without the current session actually being cleared; consider calling `_trigger_session_deleted` only when the requested conversation was the active one and has been removed.
## Individual Comments
### Comment 1
<location path="astrbot/builtin_stars/astrbot/main.py" line_range="37-42" />
<code_context>
try:
self.ltm = LongTermMemory(self.context.astrbot_config_mgr, self.context)
+
+ async def _clear_ltm_session(umo: str) -> None:
+ self.ltm.session_chats.pop(umo, None)
+ # Also clear group-level key for unique_session scenarios
+ parts = umo.split(":")
+ if len(parts) >= 3 and parts[1] == "GroupMessage":
+ group_key = f"{parts[0]}:GroupMessage:{parts[2]}"
+ self.ltm.session_chats.pop(group_key, None)
+
</code_context>
<issue_to_address>
**issue:** The group key cleanup may not align with how `_group_key` computes keys, risking stale entries.
The cleanup derives the group key by parsing `umo`, but `_group_key` builds keys from `platform_id` and `group_id` on the event instead. If `unified_msg_origin` gains more segments or its format changes, the derived key may diverge from what LongTermMemory actually used, leaving `session_chats` entries uncleared. To decouple this, either expose a helper on LongTermMemory to compute the key from `umo`/group info, or store both `umo` and the computed group key in ConversationManager and pass them to the callback for deletion.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
ConversationManager.delete_conversation,_trigger_session_deletedis called unconditionally even whenconversation_idis not the current session; consider only firing the session-deleted callbacks whencurr_cid == conversation_idto avoid clearing LTM for an actively selected conversation. - The group key parsing logic in
main._clear_ltm_sessionduplicates the format assumptions already encoded inLongTermMemory._group_key; consider centralizing this key construction/parsing (e.g., by reusing_group_keyor a shared helper) to keep behavior consistent if the key format changes in the future.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ConversationManager.delete_conversation`, `_trigger_session_deleted` is called unconditionally even when `conversation_id` is not the current session; consider only firing the session-deleted callbacks when `curr_cid == conversation_id` to avoid clearing LTM for an actively selected conversation.
- The group key parsing logic in `main._clear_ltm_session` duplicates the format assumptions already encoded in `LongTermMemory._group_key`; consider centralizing this key construction/parsing (e.g., by reusing `_group_key` or a shared helper) to keep behavior consistent if the key format changes in the future.
## Individual Comments
### Comment 1
<location path="astrbot/core/conversation_mgr.py" line_range="145" />
<code_context>
if curr_cid == conversation_id:
self.session_conversations.pop(unified_msg_origin, None)
await sp.session_remove(unified_msg_origin, "sel_conv_id")
+ await self._trigger_session_deleted(unified_msg_origin)
async def delete_conversations_by_user_id(self, unified_msg_origin: str) -> None:
</code_context>
<issue_to_address>
**issue (bug_risk):** Session-deleted hook is triggered even when the requested conversation_id is not found or not deleted
Because `await self._trigger_session_deleted(unified_msg_origin)` is outside the `if curr_cid == conversation_id` block, the hook runs on every `delete_conversation` call, even when no conversation is removed or `sel_conv_id` isn’t cleared. This may cause downstream logic (e.g., LTM cleanup) to act on a deletion that never occurred. Please only trigger this hook when a matching conversation is actually deleted, for example by moving the call inside the `if` or guarding it with a success flag.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary
修复飞书机器人对话历史管理的 3 个 bug (#8386):
delete_conversation没有触发_trigger_session_deleted回调来清理 LTM 内存。unique_session后,bot 在群聊中看不到其他用户的消息。原因是 LTM 使用unified_msg_origin(被unique_session 改写为 per-user key)作为存储 key,导致每个用户的群聊记录互相隔离。
Modifications / 改动点
conversation_mgr.py:delete_conversation末尾调用_trigger_session_deleted,与delete_conversations_by_user_id行为一致main.py: 注册 session_deleted 回调,删除对话时同步清理 LTMsession_chatsconfig.py: 删除平台适配器时级联删除该平台所有对话long_term_memory.py: 新增_group_key()方法,群聊场景下始终使用{platform_id}:GroupMessage:{group_id}作为key,不受 unique_session 影响
Test plan
Bug 1: 飞书群聊中 @bot 对话 → Web UI 删除对话 → 再次 @bot,确认不再记忆之前内容(已实测通过)
Bug 2: 删除飞书平台适配器 → 重建同 ID 适配器 → 确认旧对话不再出现
Bug 3: 开启 unique_session → 两个不同用户在群聊中发言 → @bot 确认能看到两人的消息
单元测试 5/5 通过(含 group_key 隔离验证)
相关模块已有测试无回归
This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Fix long-term memory cleanup and group chat keying for deleted conversations and unique_session scenarios.
Bug Fixes:
Tests: