-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix: scope the default session waiter identity to the sender #9753
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
icyaaaww
wants to merge
1
commit into
AstrBotDevs:master
Choose a base branch
from
icyaaaww:fix/9377-session-waiter-sender-scope
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+149
−2
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| """Tests for the session waiter's default session identity. | ||
|
|
||
| Regression coverage for the group-chat interception bug: a waiter registered by | ||
| one group member must not be triggered by a different member of the same group, | ||
| while the same member must still be isolated across different sessions. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import asyncio | ||
|
|
||
| import pytest | ||
|
|
||
| from astrbot.core.message.components import Plain | ||
| from astrbot.core.platform.astr_message_event import AstrMessageEvent | ||
| from astrbot.core.platform.astrbot_message import AstrBotMessage, MessageMember | ||
| from astrbot.core.platform.message_type import MessageType | ||
| from astrbot.core.platform.platform_metadata import PlatformMetadata | ||
| from astrbot.core.utils.session_waiter import ( | ||
| USER_SESSIONS, | ||
| DefaultSessionFilter, | ||
| SessionController, | ||
| SessionWaiter, | ||
| session_waiter, | ||
| ) | ||
|
|
||
| PLATFORM_META = PlatformMetadata( | ||
| name="aiocqhttp", | ||
| description="test platform", | ||
| id="aiocqhttp", | ||
| ) | ||
|
|
||
|
|
||
| def make_event( | ||
| sender_id: str, | ||
| session_id: str, | ||
| message_type: MessageType = MessageType.GROUP_MESSAGE, | ||
| text: str = "hello", | ||
| ) -> AstrMessageEvent: | ||
| """Build a minimal group/private message event. | ||
|
|
||
| Args: | ||
| sender_id: ID of the member that sent the message. | ||
| session_id: Platform session ID (group ID for group messages). | ||
| message_type: Message type of the event. | ||
| text: Plain text payload of the message. | ||
|
|
||
| Returns: | ||
| A usable ``AstrMessageEvent`` for session-identity assertions. | ||
| """ | ||
| message_obj = AstrBotMessage() | ||
| message_obj.type = message_type | ||
| message_obj.self_id = "bot" | ||
| message_obj.session_id = session_id | ||
| message_obj.message_id = "1" | ||
| message_obj.sender = MessageMember(user_id=sender_id, nickname=sender_id) | ||
| message_obj.message = [Plain(text=text)] | ||
| message_obj.message_str = text | ||
| message_obj.raw_message = None | ||
| if message_type == MessageType.GROUP_MESSAGE: | ||
| message_obj.group_id = session_id | ||
| return AstrMessageEvent( | ||
| message_str=text, | ||
| message_obj=message_obj, | ||
| platform_meta=PLATFORM_META, | ||
| session_id=session_id, | ||
| ) | ||
|
|
||
|
|
||
| def test_default_filter_separates_members_of_the_same_group(): | ||
| """Two members of one group must map to different session identities.""" | ||
| session_filter = DefaultSessionFilter() | ||
| event_a = make_event("member_a", "group_1") | ||
| event_b = make_event("member_b", "group_1") | ||
|
|
||
| assert session_filter.filter(event_a) != session_filter.filter(event_b) | ||
|
|
||
|
|
||
| def test_default_filter_is_stable_for_the_same_member(): | ||
| """The same member in the same group must map to one session identity.""" | ||
| session_filter = DefaultSessionFilter() | ||
| first = make_event("member_a", "group_1", text="one") | ||
| second = make_event("member_a", "group_1", text="two") | ||
|
|
||
| assert session_filter.filter(first) == session_filter.filter(second) | ||
|
|
||
|
|
||
| def test_default_filter_separates_sessions_of_the_same_member(): | ||
| """One member must not share a waiter across groups or private chats.""" | ||
| session_filter = DefaultSessionFilter() | ||
| in_group_1 = make_event("member_a", "group_1") | ||
| in_group_2 = make_event("member_a", "group_2") | ||
| in_private = make_event( | ||
| "member_a", | ||
| "member_a", | ||
| message_type=MessageType.FRIEND_MESSAGE, | ||
| ) | ||
|
|
||
| identities = { | ||
| session_filter.filter(in_group_1), | ||
| session_filter.filter(in_group_2), | ||
| session_filter.filter(in_private), | ||
| } | ||
| assert len(identities) == 3 | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_waiter_ignores_other_members_and_accepts_the_owner(): | ||
| """A registered waiter only fires for the member that created it.""" | ||
| USER_SESSIONS.clear() | ||
| session_filter = DefaultSessionFilter() | ||
| owner_event = make_event("member_a", "group_1", text="@bot") | ||
| other_event = make_event("member_b", "group_1", text="unrelated chatter") | ||
|
|
||
| triggered: list[str] = [] | ||
|
|
||
| @session_waiter(timeout=5) | ||
| async def waiter(controller: SessionController, event: AstrMessageEvent) -> None: | ||
| triggered.append(event.get_sender_id()) | ||
| controller.stop() | ||
|
|
||
| waiting = asyncio.create_task(waiter(owner_event, session_filter)) | ||
| await asyncio.sleep(0) | ||
|
|
||
| # A different member of the same group must not reach the waiter. | ||
| await SessionWaiter.trigger(session_filter.filter(other_event), other_event) | ||
| assert triggered == [] | ||
|
|
||
| # The owner's own follow-up message must reach the waiter. | ||
| follow_up = make_event("member_a", "group_1", text="the real question") | ||
| await SessionWaiter.trigger(session_filter.filter(follow_up), follow_up) | ||
| await asyncio.wait_for(waiting, timeout=5) | ||
|
|
||
| assert triggered == ["member_a"] | ||
| assert USER_SESSIONS == {} | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (testing): Add coverage for the timeout/cleanup path of SessionWaiter
This test only covers the success path where the owner’s follow-up arrives and the waiter fires, ending with
USER_SESSIONSempty. Please add a separate async test for the timeout path: register a waiter with a short timeout, ensure no matching follow-up arrives, then assert that after the timeout (1) the waiter is removed fromUSER_SESSIONS, and (2) non‑owner messages during the wait do not trigger the waiter. That will exercise both success and timeout branches.Suggested implementation:
asynciois imported at the top oftests/unit/test_session_waiter.py:SessionWaitertests (e.g., immediately aftertest_waiter_ignores_other_members_and_accepts_the_owner) to keep related coverage together. If the indentation in the snippet is only due to doc formatting, this patch will already do that; otherwise, you may need to re-indent the@session_waiter(timeout=5)line so it remains at the correct scope.session_waiterregistration semantics differ (e.g., if the first call does not register the waiter), adjust the test to use the correct way of creating and registering a waiter so thatUSER_SESSIONSis populated for"member_a"and then cleaned up on timeout.