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.