fix(session_waiter): bind empty-mention waiter to the initiating sender (#9377)

[Qixuan112]

Summary

Fixes #9377

Problem

When a user sent an empty @mention (or wake-prefix-only message) in a group chat, the empty_mention_waiter used DefaultSessionFilter which keyed sessions only by event.unified_msg_origin (the group/conversation ID). This meant any subsequent message from any member in that group would trigger the waiter and be consumed via event.stop_event(), causing other members' messages to be silently intercepted.

Fix

Added a custom _SenderSessionFilter that includes event.get_sender_id() in the session key:

class _SenderSessionFilter(SessionFilter):
    def filter(self, ev: AstrMessageEvent) -> str:
        return f"{ev.unified_msg_origin}:{ev.get_sender_id()}"

This ensures the waiter only responds to messages from the same sender who initiated the empty mention, matching expected multi-user group chat behavior.

Changes

• astrbot/builtin_stars/astrbot/main.py: imported SessionFilter, added a sender-scoped filter for empty_mention_waiter (+6/-1 lines)

Summary by Sourcery

Bug Fixes:

• Ensure empty-mention waiters in group chats only consume follow-up messages from the original sender instead of any group member.

[sourcery-ai]

Hey - I've left some high level feedback:

• Consider moving _SenderSessionFilter out of handle_empty_mention into module scope so it isn’t redefined on every call and can be reused or tested independently.
• When building the session key in _SenderSessionFilter.filter, consider explicitly handling cases where get_sender_id() might be None or unexpected to avoid collisions or hard-to-debug behavior.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Consider moving `_SenderSessionFilter` out of `handle_empty_mention` into module scope so it isn’t redefined on every call and can be reused or tested independently.
- When building the session key in `_SenderSessionFilter.filter`, consider explicitly handling cases where `get_sender_id()` might be `None` or unexpected to avoid collisions or hard-to-debug behavior.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[Qixuan112]

Addressed both suggestions in dfd6c00:

• Moved _SenderSessionFilter to module scope so it's defined once and independently testable.
• Guarded against empty/falsy sender_id in filter() by substituting a sentinel (__unknown_sender__), avoiding accidental key collisions if a platform fails to supply a sender ID.

(🤖 Addressed by Claude Code)

[xiaoyuyu6420]

Review

Reviewed in the context of issue #9377 alongside the other open PRs (#9378, #9442).

Strengths

1. Minimal diff — only touches main.py, adds a small private class. Lowest risk of unintended side effects.
2. Private _SenderSessionFilter — scoped to the module, no new public API surface to maintain.
3. __unknown_sender__ sentinel — degrades gracefully when sender_id is unavailable instead of skipping the waiter entirely.

Issues

1. No tests

This is a bug fix with zero test coverage. Given that two other PRs for the same issue provide extensive tests, this is a significant gap. At minimum, the following should be tested:

• Two senders in the same group produce different keys
• The original sender's follow-up is still captured
• Cross-conversation isolation
• __unknown_sender__ sentinel behavior

2. Does not fix the non-text message swallowing bug (mentioned in #9377 as a "连带问题")

The handler body is unchanged:

if not event.message_str or not event.message_str.strip():
    return  # pure images are still silently dropped

3. Does not fix the _cleanup race condition (mentioned in #9377)

USER_SESSIONS.pop(self.session_id) in _cleanup() can evict a newer waiter registered under the same key.

4. Only fixes empty_mention_waiter, not is_wake_prefix_only

The issue report states the cross-user interception also applies to the wake-prefix-only path, which still uses DefaultSessionFilter.

5. __unknown_sender__ could collide with a real sender_id

While extremely unlikely, a platform could theoretically return "__unknown_sender__" as an actual user ID. A value like "<unknown>" (with angle brackets) or a UUID-based sentinel would be safer since real IDs never contain brackets. Alternatively, PR #9378's approach of skipping the waiter (with a log warning) avoids the collision entirely.

6. Key collision with : separator

The format f"{ev.unified_msg_origin}:{sender_id}" uses : as separator, but unified_msg_origin already contains : (e.g., napcat2:GroupMessage:984306989). While in practice this does not cause ambiguity because the format is fixed (platform:Type:id), a Telegram topic-group origin like telegram:GroupMessage:-100123#42 combined with a sender ID containing : could theoretically produce collisions. PR #9378 addresses this with length-prefixed encoding.

Summary

This is the most minimal fix for the core cross-user interception bug, but it lacks tests and does not address the two related issues (non-text swallowing, cleanup race) documented in #9377. The sentinel value and key format have minor theoretical concerns that are unlikely to cause problems in practice but worth noting.

[xiaoyuyu6420]

Correction to my earlier review

I need to correct issue #4 in my previous review. I wrote that the is_wake_prefix_only path still uses DefaultSessionFilter, but this is factually wrong.

After re-reading the control flow: both is_empty_mention and is_wake_prefix_only branches merge after the guard at line 72 and reach the single empty_mention_waiter(event, session_filter=_SenderSessionFilter()) call. There is no second call site. The wake-prefix path is covered.

Thanks to @hedssaz's response on #9378 for catching this — I made the same error in both reviews. The other points (missing tests, non-text swallowing, cleanup race) still stand.

[Qixuan112]

@xiaoyuyu6420 感谢详细审查与自我更正。已逐项处理并推送：

1. 测试 → 已补齐

新增 tests/unit/test_empty_mention_sender_filter.py，13 个用例覆盖你要求的全部场景：

• 同群不同发送者 → 不同 key
• 原发送者 follow-up 仍被捕获（真实走 @session_waiter + trigger 机制）
• 跨会话隔离（同发送者跨群不命中）
• 哨兵行为：""/None/0 均回落 "<unknown>" 哨兵
• 空白 follow-up 不消耗 waiter
  全部 13 用例通过（strict asyncio 模式）。

5. 哨兵碰撞 → 已修复

采纳你的建议：哨兵从 "__unknown_sender__" 改为 "<unknown>"（带尖括号）。已核验全仓库所有平台适配器的 sender_id 均不含尖括号，因此真实用户 id 不可能与未知发送者的 key 碰撞。新增测试 test_real_sender_id_equal_to_sentinel_does_not_collide 专门锁定"真实 id 恰好等于旧哨兵字符串也不碰撞"这一行为。

4. wake-prefix 路径 → 确认已覆盖（与你的更正一致）

独立核验：is_empty_mention 与 is_wake_prefix_only 分支合并后共用唯一的 _SenderSessionFilter() 调用点（main.py:147），_SenderSessionFilter( 在 astrbot/ 下仅此一处实例化，wake-prefix 路径同样绑定发起者。

6. : 分隔 key → 保持现状

维持 f"{umo}:{sender_id}" 格式。unified_msg_origin 解析用 maxsplit=2，且该 key 仅作 USER_SESSIONS 字典键、无回解析风险，实践中无歧义。不做长度前缀编码（过度工程）。

2/3. 非文本消息被吞、_cleanup 竞态 → 超出本 PR 范围

确认均为 #9377 中列出的连带问题，且是本 PR 之前就存在的行为（本 PR 未触碰 session_waiter.py 任何代码）。建议作为独立 issue/PR 跟进。

🤖 Prepared by Claude Code