From 6a2a871e655eae7d8975f6b49708d773f7b471d5 Mon Sep 17 00:00:00 2001 From: Qixuan112 <3573568193@qq.com> Date: Tue, 28 Jul 2026 10:35:09 +0800 Subject: [PATCH 1/4] fix(session_waiter): bind empty-mention waiter to the initiating sender to avoid intercepting other group members (#9377) --- astrbot/builtin_stars/astrbot/main.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/astrbot/builtin_stars/astrbot/main.py b/astrbot/builtin_stars/astrbot/main.py index e0053a29de..accb122856 100644 --- a/astrbot/builtin_stars/astrbot/main.py +++ b/astrbot/builtin_stars/astrbot/main.py @@ -13,6 +13,7 @@ FILTERS, USER_SESSIONS, SessionController, + SessionFilter, SessionWaiter, session_waiter, ) @@ -109,6 +110,10 @@ async def handle_empty_mention(self, event: AstrMessageEvent): logger.error(f"LLM response failed: {e!s}") yield event.plain_result("想要问什么呢?😄") + class _SenderSessionFilter(SessionFilter): + def filter(self, ev: AstrMessageEvent) -> str: + return f"{ev.unified_msg_origin}:{ev.get_sender_id()}" + @session_waiter(60) async def empty_mention_waiter( controller: SessionController, @@ -126,7 +131,7 @@ async def empty_mention_waiter( controller.stop() try: - await empty_mention_waiter(event) + await empty_mention_waiter(event, session_filter=_SenderSessionFilter()) except TimeoutError: pass except Exception as e: From dfd6c00183702064a5dec98e9a014269e594183e Mon Sep 17 00:00:00 2001 From: Qixuan112 <3573568193@qq.com> Date: Tue, 28 Jul 2026 10:41:26 +0800 Subject: [PATCH 2/4] refactor: move _SenderSessionFilter to module scope and guard against empty sender_id --- astrbot/builtin_stars/astrbot/main.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/astrbot/builtin_stars/astrbot/main.py b/astrbot/builtin_stars/astrbot/main.py index accb122856..d9f18a4bba 100644 --- a/astrbot/builtin_stars/astrbot/main.py +++ b/astrbot/builtin_stars/astrbot/main.py @@ -28,6 +28,22 @@ def _iter_message_components(event: AstrMessageEvent): return tuple(messages) +class _SenderSessionFilter(SessionFilter): + """Session filter scoped to a specific (conversation, sender) pair. + + Used by the empty-mention waiter so that only the user who initiated the + empty mention (not every member of the group) can satisfy the waiter. + """ + + def filter(self, ev: AstrMessageEvent) -> str: + sender_id = ev.get_sender_id() or "" + if not sender_id: + # Fall back to a value that will not accidentally collide with a + # legitimate session key from another member. + sender_id = "__unknown_sender__" + return f"{ev.unified_msg_origin}:{sender_id}" + + class Main(star.Star): def __init__(self, context: star.Context) -> None: self.context = context @@ -110,10 +126,6 @@ async def handle_empty_mention(self, event: AstrMessageEvent): logger.error(f"LLM response failed: {e!s}") yield event.plain_result("想要问什么呢?😄") - class _SenderSessionFilter(SessionFilter): - def filter(self, ev: AstrMessageEvent) -> str: - return f"{ev.unified_msg_origin}:{ev.get_sender_id()}" - @session_waiter(60) async def empty_mention_waiter( controller: SessionController, From 0f800e4e1e08aed27d75a6eb9a2dc9c4c21ad271 Mon Sep 17 00:00:00 2001 From: Qixuan112 <3573568193@qq.com> Date: Sat, 1 Aug 2026 00:13:36 +0800 Subject: [PATCH 3/4] test(empty_mention): add unit tests for sender-scoped session filter Add locked tests covering the sender-binding fix in #9422 / #9377: - Different senders in the same group produce different session keys - Cross-session isolation: same sender in different groups differ - Repeatability: same (group, sender) maps to the same key - Falsy sender ids ("", None, 0) fall back to the "" sentinel - A real sender id equal to the legacy sentinel string does not collide with the unknown-sender fallback key - Integration: the original sender's follow-up is captured, other group members' messages are not, cross-conversation messages are not, and blank follow-ups do not satisfy the waiter These tests encode the expected behavior and serve as the acceptance contract for the sender-scoped waiter. --- .../unit/test_empty_mention_sender_filter.py | 324 ++++++++++++++++++ 1 file changed, 324 insertions(+) create mode 100644 tests/unit/test_empty_mention_sender_filter.py diff --git a/tests/unit/test_empty_mention_sender_filter.py b/tests/unit/test_empty_mention_sender_filter.py new file mode 100644 index 0000000000..d615af2e1d --- /dev/null +++ b/tests/unit/test_empty_mention_sender_filter.py @@ -0,0 +1,324 @@ +"""Tests for `_SenderSessionFilter` and the empty-mention waiter sender binding. + +Covers PR #9422 / issue #9377: the empty-mention waiter must be scoped to the +(session, sender) pair that initiated it, so that in a group chat only the +initiating member can satisfy the waiter -- not every member of the group. + +Two layers are exercised: + +A. `_SenderSessionFilter.filter()` in isolation (pure key computation). +B. Integration through the `session_waiter` decorator + `SessionWaiter.trigger`, + mirroring how `Main.handle_session_control_agent` dispatches follow-ups. +""" + +import asyncio +from unittest.mock import MagicMock + +import pytest + +from astrbot.builtin_stars.astrbot.main import _SenderSessionFilter +from astrbot.core.utils.session_waiter import ( + FILTERS, + USER_SESSIONS, + SessionWaiter, + session_waiter, +) + +# Target sentinel for unresolvable sender ids (locked by review). Uses angle +# brackets because no platform produces a real sender id containing them. +UNKNOWN_SENDER_SENTINEL = "" + +UMO_GROUP_A = "test_platform:group:111" +UMO_GROUP_B = "test_platform:group:222" + +DEFAULT_MESSAGE = "hello" + + +def make_event(umo: str, sender_id) -> MagicMock: + """Build a minimal mock AstrMessageEvent. + + Only the surface used by the filter and the waiter machinery is populated: + `unified_msg_origin`, `get_sender_id()` and `message_str`. + """ + ev = MagicMock() + ev.unified_msg_origin = umo + ev.get_sender_id.return_value = sender_id + ev.message_str = "" + return ev + + +@pytest.fixture(autouse=True) +def _clean_global_session_state(): + """Never leak USER_SESSIONS / FILTERS across tests.""" + USER_SESSIONS.clear() + FILTERS.clear() + yield + USER_SESSIONS.clear() + FILTERS.clear() + + +async def _wait_for_registration(session_id: str, timeout: float = 1.0): + """Poll until a waiter with `session_id` is registered; return its controller.""" + async def _poll(): + while session_id not in USER_SESSIONS: + await asyncio.sleep(0.01) + + await asyncio.wait_for(_poll(), timeout=timeout) + return USER_SESSIONS[session_id].session_controller + + +def _release_keep_watcher(session_controller) -> None: + """Unblock the `SessionController.keep()` background watcher task so the + test event loop has no dangling tasks after an early completion.""" + if session_controller.current_event is not None: + session_controller.current_event.set() + + +async def _dispatch_like_control_agent(event) -> None: + """Mirror `Main.handle_session_control_agent`: for every registered filter, + compute the session key and trigger any matching waiter.""" + for session_filter in list(FILTERS): + session_id = session_filter.filter(event) + if session_id in USER_SESSIONS: + await SessionWaiter.trigger(session_id, event) + + +# ===================================================================== +# A. _SenderSessionFilter.filter() -- pure unit tests +# ===================================================================== + + +def test_different_senders_in_same_group_produce_different_keys(): + """A follow-up from another member of the same group must map to a + different session key than the initiator's.""" + f = _SenderSessionFilter() + ev_a = make_event(UMO_GROUP_A, "10001") + ev_b = make_event(UMO_GROUP_A, "10002") + assert f.filter(ev_a) != f.filter(ev_b) + + +def test_same_sender_in_different_groups_produce_different_keys(): + """Cross-session isolation: the same sender in another conversation must + not share a session key.""" + f = _SenderSessionFilter() + ev_g1 = make_event(UMO_GROUP_A, "10001") + ev_g2 = make_event(UMO_GROUP_B, "10001") + assert f.filter(ev_g1) != f.filter(ev_g2) + + +def test_same_sender_same_group_produces_same_key(): + """Repeatability: the initiator's follow-ups keep mapping to the same key.""" + f = _SenderSessionFilter() + ev_1 = make_event(UMO_GROUP_A, "10001") + ev_2 = make_event(UMO_GROUP_A, "10001") + assert f.filter(ev_1) == f.filter(ev_2) == f"{UMO_GROUP_A}:10001" + + +@pytest.mark.parametrize("falsy_sender_id", ["", None, 0]) +def test_falsy_sender_id_maps_to_unknown_sender_sentinel(falsy_sender_id): + """An empty/falsy sender id falls back to a sentinel so the waiter never + binds to the raw conversation alone.""" + f = _SenderSessionFilter() + ev = make_event(UMO_GROUP_A, falsy_sender_id) + assert f.filter(ev) == f"{UMO_GROUP_A}:{UNKNOWN_SENDER_SENTINEL}" + # The sentinel key must not collide with a legitimate member's key. + ev_real = make_event(UMO_GROUP_A, "10001") + assert f.filter(ev) != f.filter(ev_real) + + +def test_two_unknown_senders_in_same_group_share_key(): + """Documented limitation of the current fallback: all members without a + resolvable sender id collapse onto the same key in a group (they can still + hijack each other's waiter). Kept as a documentation test, not as the + desired behavior.""" + f = _SenderSessionFilter() + ev_u1 = make_event(UMO_GROUP_A, "") + ev_u2 = make_event(UMO_GROUP_A, None) + assert f.filter(ev_u1) == f.filter(ev_u2) + + +def test_real_sender_id_equal_to_sentinel_does_not_collide(): + """A genuine sender id must never collide with the unknown-sender fallback + key. + + Target behavior (locked by review): the sentinel is '', which no + real sender id contains, so even a real id that exactly equals the legacy + sentinel string '__unknown_sender__' maps to its own distinct key. + """ + f = _SenderSessionFilter() + ev_unknown = make_event(UMO_GROUP_A, "") + ev_legacy_sentinel_id = make_event(UMO_GROUP_A, "__unknown_sender__") + assert f.filter(ev_unknown) != f.filter(ev_legacy_sentinel_id) + + +# ===================================================================== +# B. Integration: the waiter is bound to the initiating sender +# ===================================================================== + + +@pytest.mark.asyncio +async def test_original_sender_followup_is_captured(): + """The initiator's follow-up in the same group satisfies the waiter.""" + handler_called = asyncio.Event() + handled = [] + + @session_waiter(timeout=1.0) + async def waiter(controller, event): + if not event.message_str or not event.message_str.strip(): + return + handled.append(event) + handler_called.set() + controller.stop() + + init_event = make_event(UMO_GROUP_A, "sender_a") + followup_event = make_event(UMO_GROUP_A, "sender_a") + followup_event.message_str = DEFAULT_MESSAGE + + session_id = _SenderSessionFilter().filter(init_event) + waiter_task = asyncio.create_task( + waiter(init_event, session_filter=_SenderSessionFilter()) + ) + controller = await _wait_for_registration(session_id) + + await _dispatch_like_control_agent(followup_event) + await asyncio.wait_for(waiter_task, timeout=1.0) + + assert handler_called.is_set() + assert len(handled) == 1 + assert session_id not in USER_SESSIONS # cleaned up after completion + _release_keep_watcher(controller) + + +@pytest.mark.asyncio +async def test_other_member_followup_is_not_captured(): + """A different member of the same group must NOT satisfy the waiter (the + regression this PR fixes).""" + handler_called = asyncio.Event() + handled = [] + + @session_waiter(timeout=0.3) + async def waiter(controller, event): + handled.append(event) + handler_called.set() + controller.stop() + + init_event = make_event(UMO_GROUP_A, "sender_a") + other_event = make_event(UMO_GROUP_A, "sender_b") + other_event.message_str = DEFAULT_MESSAGE + + session_id_a = _SenderSessionFilter().filter(init_event) + session_id_b = _SenderSessionFilter().filter(other_event) + assert session_id_a != session_id_b + + waiter_task = asyncio.create_task( + waiter(init_event, session_filter=_SenderSessionFilter()) + ) + await _wait_for_registration(session_id_a) + + await _dispatch_like_control_agent(other_event) + await asyncio.sleep(0.05) + + assert not handler_called.is_set() + assert len(handled) == 0 + # The waiter is still bound to sender_a and still waiting. + assert session_id_a in USER_SESSIONS + assert session_id_b not in USER_SESSIONS + + # Nobody hijacked it; it eventually times out on its own. + with pytest.raises(TimeoutError): + await asyncio.wait_for(waiter_task, timeout=1.0) + + +@pytest.mark.asyncio +async def test_cross_session_other_group_followup_is_not_captured(): + """The same sender in a different conversation must NOT satisfy the waiter.""" + handler_called = asyncio.Event() + + @session_waiter(timeout=0.3) + async def waiter(controller, event): + handler_called.set() + controller.stop() + + init_event = make_event(UMO_GROUP_A, "sender_a") + cross_event = make_event(UMO_GROUP_B, "sender_a") + cross_event.message_str = DEFAULT_MESSAGE + + session_id_a = _SenderSessionFilter().filter(init_event) + + waiter_task = asyncio.create_task( + waiter(init_event, session_filter=_SenderSessionFilter()) + ) + await _wait_for_registration(session_id_a) + + await _dispatch_like_control_agent(cross_event) + await asyncio.sleep(0.05) + + assert not handler_called.is_set() + assert session_id_a in USER_SESSIONS + + with pytest.raises(TimeoutError): + await asyncio.wait_for(waiter_task, timeout=1.0) + + +@pytest.mark.asyncio +async def test_blank_followup_does_not_satisfy_waiter(): + """A blank follow-up from the initiator returns early without stopping the + controller, matching `empty_mention_waiter`'s blank-message guard.""" + handler_called = asyncio.Event() + + @session_waiter(timeout=0.3) + async def waiter(controller, event): + if not event.message_str or not event.message_str.strip(): + return + handler_called.set() + controller.stop() + + init_event = make_event(UMO_GROUP_A, "sender_a") + blank_event = make_event(UMO_GROUP_A, "sender_a") + blank_event.message_str = " " + + session_id = _SenderSessionFilter().filter(init_event) + + waiter_task = asyncio.create_task( + waiter(init_event, session_filter=_SenderSessionFilter()) + ) + await _wait_for_registration(session_id) + + await _dispatch_like_control_agent(blank_event) + await asyncio.sleep(0.05) + + assert not handler_called.is_set() + assert session_id in USER_SESSIONS + + with pytest.raises(TimeoutError): + await asyncio.wait_for(waiter_task, timeout=1.0) + + +@pytest.mark.asyncio +async def test_unknown_sender_registration_and_trigger_use_same_key(): + """The unknown-sender fallback still pairs registration and follow-up via + the sentinel key within the same group.""" + handler_called = asyncio.Event() + + @session_waiter(timeout=1.0) + async def waiter(controller, event): + handler_called.set() + controller.stop() + + unknown_init = make_event(UMO_GROUP_A, "") + unknown_followup = make_event(UMO_GROUP_A, "") + unknown_followup.message_str = DEFAULT_MESSAGE + + session_id = _SenderSessionFilter().filter(unknown_init) + assert session_id == f"{UMO_GROUP_A}:{UNKNOWN_SENDER_SENTINEL}" + + waiter_task = asyncio.create_task( + waiter(unknown_init, session_filter=_SenderSessionFilter()) + ) + controller = await _wait_for_registration(session_id) + + await _dispatch_like_control_agent(unknown_followup) + await asyncio.wait_for(waiter_task, timeout=1.0) + + assert handler_called.is_set() + _release_keep_watcher(controller) From 074edb716b9bd00baca8b816312cd23b795c4887 Mon Sep 17 00:00:00 2001 From: Qixuan112 <3573568193@qq.com> Date: Sat, 1 Aug 2026 00:13:36 +0800 Subject: [PATCH 4/4] fix(empty_mention): use angle-bracket sentinel for unknown sender id Address review feedback #5 on #9422: the previous "__unknown_sender__" sentinel could collide with a real platform sender id of the same string, letting that member still hijack another user's empty-mention waiter. Switch the fallback to "": angle brackets cannot appear in any real platform sender id, so an unknown sender can never share a session key with a real member. The sentinel value is locked by the unit tests. Review items #4 (wake-prefix path) and #6 (':' separator) are confirmed already covered; #2/#3 are related issues from #9377 outside this PR's minimal scope. --- astrbot/builtin_stars/astrbot/main.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/astrbot/builtin_stars/astrbot/main.py b/astrbot/builtin_stars/astrbot/main.py index d9f18a4bba..e4b4edc079 100644 --- a/astrbot/builtin_stars/astrbot/main.py +++ b/astrbot/builtin_stars/astrbot/main.py @@ -38,9 +38,10 @@ class _SenderSessionFilter(SessionFilter): def filter(self, ev: AstrMessageEvent) -> str: sender_id = ev.get_sender_id() or "" if not sender_id: - # Fall back to a value that will not accidentally collide with a - # legitimate session key from another member. - sender_id = "__unknown_sender__" + # Fall back to a sentinel that will not accidentally collide with a + # legitimate session key from another member. Angle brackets are + # used because no real platform sender id contains them. + sender_id = "" return f"{ev.unified_msg_origin}:{sender_id}"