ソースを参照

Stop trusting member_user_ids to decide whether a DM exists

member_user_ids is not among the fields the ListRooms documentation lists for
RoomWithViewerState, so a server may leave it empty — yet two places treated an
empty field as "this room has no members": the login search skipped the room
outright, and _find_dm_room concluded no DM exists. Both turned a DM the bot
can plainly see into "shares no DM with the bot", which sends the user looking
for a message they never needed to send.

_find_dm_room now falls back to a ListMembers RPC per DM room when the
directory reports no membership (one extra call, only on servers that need it),
and the login search stops gating on the field. Everywhere else in the adapter
membership already came from list_room_members, so this also removes the one
place that trusted the directory for it.
Paul Klumpp 2 週間 前
親
コミット
44e81e6364
2 ファイル変更、74 行追加、5 行削除
  1. 27 5
      adapter.py
  2. 47 0
      test_adapter.py

+ 27 - 5
adapter.py

@@ -3808,10 +3808,14 @@ async def _find_user_by_login(client: ChattoClient, user_login: str) -> tuple[st
     the channels the bot sits in would cost one member RPC per room and could
     resolve a login whose DM does not exist yet. A bot cannot read the
     server-wide user directory either: that needs admin rights.
+
+    Every DM room is searched, including one whose membership the directory
+    left unreported — that field is optional, and skipping such a room would
+    make its login unfindable while the DM room sits right there.
     """
     for rws in await client.list_rooms():
         room = rws.room
-        if room is None or room.kind != RoomKind.DM or not rws.member_user_ids:
+        if room is None or room.kind != RoomKind.DM:
             continue
         try:
             member_ids, _ = await client.list_room_members(room.id, search=user_login)
@@ -3836,14 +3840,32 @@ async def _find_user_by_login(client: ChattoClient, user_login: str) -> tuple[st
 async def _find_dm_room(
     client: ChattoClient, our_id: str, other_id: str
 ) -> Room | None:
-    """The DM room between two users, or None when there is none yet."""
+    """The DM room between two users, or None when there is none yet.
+
+    Membership is read from the room object the directory already returned, and
+    only falls back to a ListMembers RPC when that came back empty:
+    ``member_user_ids`` is not among the fields the ListRooms documentation
+    lists for ``RoomWithViewerState``, so a server is free to leave it
+    unpopulated — and a DM room we can see plainly exists, whatever the
+    directory said about who is in it. The fallback costs one RPC per DM room
+    and only on servers that need it.
+    """
     for rws in await client.list_rooms():
         room = rws.room
         if room is None or room.kind != RoomKind.DM:
             continue
-        # The directory reports membership on the room object; a second
-        # ListMembers RPC per DM room would buy nothing.
-        if other_id in rws.member_user_ids and our_id in rws.member_user_ids:
+        if rws.member_user_ids:
+            if other_id in rws.member_user_ids and our_id in rws.member_user_ids:
+                return room
+            continue
+        try:
+            member_ids, _page = await client.list_room_members(room.id)
+        except ChattoError as exc:
+            # An unreadable room cannot be the one we want, but it must not
+            # abort the search over the remaining ones.
+            logger.debug("Chatto tools: members of DM %s unreadable: %s", room.id, exc)
+            continue
+        if other_id in member_ids and our_id in member_ids:
             return room
     return None
 

+ 47 - 0
test_adapter.py

@@ -3550,6 +3550,53 @@ class TestPluginToolHandlers:
         assert result["success"] is True
         assert result["dm_room_id"] == "dm-user-1"
 
+    async def test_send_dm_finds_a_dm_room_the_directory_omits_members_for(self):
+        """`member_user_ids` is not part of the documented ListRooms response, so
+        a server may leave it empty. A DM room the bot can see plainly exists,
+        and refusing with "no DM exists yet" would send the user looking for a
+        message they never have to send.
+        """
+        client = self._stub_client()
+        client.list_rooms = AsyncMock(
+            return_value=[_make_room_state(_make_room("dm-user-1", "", RoomKind.DM))]
+        )
+        client.list_room_members = AsyncMock(
+            return_value=(["bot-user-id", "user-1"], Page())
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_send_dm,
+            {"user_id": "user-1", "message": "hi"},
+            client=client,
+        )
+        assert result["success"] is True
+        assert result["dm_room_id"] == "dm-user-1"
+
+    async def test_send_dm_resolves_a_login_the_directory_omits_members_for(self):
+        """The same empty field also gated the login search: a room whose
+        membership the directory did not report was skipped outright, so the
+        login was unfindable even though the DM room sits right there."""
+        client = self._stub_client()
+        client.list_rooms = AsyncMock(
+            return_value=[_make_room_state(_make_room("dm-user-1", "", RoomKind.DM))]
+        )
+
+        # A search narrows the result; an unsearched call lists the whole
+        # room, which is what the DM-room lookup then asks for.
+        async def _members(room_id, *, search="", **kwargs):
+            return (["user-1"] if search else ["bot-user-id", "user-1"], Page())
+
+        client.list_room_members = AsyncMock(side_effect=_members)
+        client.batch_get_room_members = AsyncMock(
+            return_value=[DirectoryMember(user=_make_user("user-1", "Alice"))]
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_send_dm,
+            {"user_login": "alice", "message": "hi"},
+            client=client,
+        )
+        assert result["success"] is True
+        assert result["recipient_user_id"] == "user-1"
+
     async def test_send_dm_resolves_a_login_in_the_shared_dm_room(self):
         """A login is looked up in the rooms we share, not in an admin-only
         server-wide directory the bot cannot read."""