Matrix DM room misclassification fixes and detection logic

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #51731 fix(matrix): use member_count as DM signal for named DM rooms (#48551)

  merged to main

  files: plugins/platforms/matrix/adapter.py, tests/gateway/test_matrix.py, tests/gateway/test_matrix_project_context_isolation.py

  Show the diff

  • plugins/platforms/matrix/adapter.py

    @@ -3573,21 +3573,29 @@ def _state_event_value(event: Any, key: str) -> Optional[str]:
             return None
     
         async def _get_room_member_count(self, room_id: str) -> Optional[int]:
    +        # Tier 1: state_store (fast, cache-backed).
             state_store = (
                 getattr(self._client, "state_store", None) if self._client else None
             )
    -        if not state_store:
    -            return None
    -        try:
    -            members = await state_store.get_members(room_id)
    -        except Exception:
    -            return None
    -        if members is None:
    -            return None
    -        try:
    -            return len(members)
    -        except TypeError:
    -            return None
    +        if state_store:
    +            try:
    +                members = await state_store.get_members(room_id)
    +                if members is not None:
    +                    return len(members)
    +            except Exception:
    +                pass
    +
    +        # Tier 2: API fallback (direct server query) when the cache is empty.
    +        client = getattr(self, "_client", None)
    +        if client is not None and hasattr(client, "joined_members"):
    +            try:
    +                resp = await client.joined_members(room_id)
    +                if getattr(resp, "members", None) is not None:
    +                    return len(resp.members)
    +            except Exception:
    +                pass
    +
    +        return None
     
         async def _get_room_name(self, room_id: str) -> Optional[str]:
             if not self._client or not hasattr(self._client, "get_state_event"):
    @@ -3674,8 +3682,23 @@ async def _resolve_room_identity(
             member_count = await self._get_room_member_count(room_id)
             has_explicit_name = bool(room_name)
             is_direct = bool(self._dm_rooms.get(room_id, False))
    -        conflict = bool(is_direct and has_explicit_name)
    -        chat_type = "dm" if is_direct and not has_explicit_name else "room"
    +        # member_count is the primary DM signal: <=2 members means this is
    +        # necessarily a 1:1 conversation (or self-DM), regardless of m.direct
    +        # or room name. Most Matrix clients auto-name DM rooms (e.g.
    +        # "Alice & Bot"), so the old `not has_explicit_name` check
    +        # misclassified virtually all client-created DMs as rooms. Falls back
    +        # to the m.direct + name heuristic when the count is unavailable (e.g.
    +        # state_store and API query both fail). A room that grew to 3+ members
    +        # but is still in stale m.direct is correctly classified as a room.
    +        is_likely_dm = (member_count is not None and member_count <= 2) or (
    +            is_direct and not has_explicit_name
    +        )
    +        conflict = bool(
    +            is_direct
    +            and has_explicit_name
    +            and (member_count is None or member_count > 2)
    +        )
    +        chat_type = "dm" if is_likely_dm else "room"
             display_name = room_name or canonical_alias or room_id
     
             identity = MatrixRoomIdentity(

  • tests/gateway/test_matrix.py

    @@ -502,8 +502,9 @@ async def test_m_direct_room_is_dm(self):
             assert await self.adapter._is_dm_room("!dm_room:ex.org") is True
     
         @pytest.mark.asyncio
    -    async def test_named_two_member_room_is_not_dm(self):
    -        """A named two-member room must remain a room, not a DM."""
    +    async def test_named_two_member_room_is_dm_by_member_count(self):
    +        """A named two-member room NOT in m.direct is treated as a DM because
    +        <=2 members means it's necessarily a 1:1 conversation."""
             self.adapter._joined_rooms = {"!project:ex.org"}
             self.adapter._dm_rooms = {}
             self.adapter._client = MagicMock()
    @@ -519,14 +520,41 @@ async def test_named_two_member_room_is_not_dm(self):
     
             identity = await self.adapter._resolve_room_identity("!project:ex.org")
     
    -        assert identity.chat_type == "room"
    +        assert identity.chat_type == "dm"
             assert identity.display_name == "Project Room"
             assert identity.joined_member_count == 2
    -        assert await self.adapter._is_dm_room("!project:ex.org") is False
    +        assert await self.adapter._is_dm_room("!project:ex.org") is True
    +
    +    @pytest.mark.asyncio
    +    async def test_named_two_member_dm_is_dm(self):
    +        """A named two-member room in m.direct is a DM (not a room).
    +
    +        Most Matrix clients auto-name DM rooms (e.g. "Alice & Bot"), so the
    +        old `not has_explicit_name` override misclassified them as rooms.
    +        """
    +        self.adapter._joined_rooms = {"!named_dm:ex.org"}
    +        self.adapter._dm_rooms = {"!named_dm:ex.org": True}
    +        self.adapter._client = MagicMock()
    +        self.adapter._client.get_state_event = AsyncMock(
    +            side_effect=lambda room_id, event_type: {"name": "Alice & Bot"}
    +            if event_type == "m.room.name"
    +            else (_ for _ in ()).throw(Exception("no alias"))
    +        )
    +        self.adapter._client.state_store = MagicMock()
    +        self.adapter._client.state_store.get_members = AsyncMock(
    +            return_value=["@bot:ex.org", "@alice:ex.org"]
    +        )
    +
    +        identity = await self.adapter._resolve_room_identity("!named_dm:ex.org")
    +
    +        assert identity.chat_type == "dm"
    +        assert identity.conflict is False
    +        assert identity.joined_member_count == 2
    +        assert await self.adapter._is_dm_room("!named_dm:ex.org") is True
     
         @pytest.mark.asyncio
         async def test_named_room_overrides_stale_dm_cache(self):
    -        """Explicit room names should defeat stale/conflicting m.direct data."""
    +        """Explicit room names defeat stale/conflicting m.direct data when 3+ members."""
             self.adapter._joined_rooms = {"!stale:ex.org"}
             self.adapter._dm_rooms = {"!stale:ex.org": True}
             self.adapter._client = MagicMock()
    @@ -536,12 +564,15 @@ async def test_named_room_overrides_stale_dm_cache(self):
                 else (_ for _ in ()).throw(Exception("no alias"))
             )
             self.adapter._client.state_store = MagicMock()
    -        self.adapter._client.state_store.get_members = AsyncMock(return_value=["@bot:ex.org", "@alice:ex.org"])
    +        self.adapter._client.state_store.get_members = AsyncMock(
    +            return_value=["@bot:ex.org", "@alice:ex.org", "@bob:ex.org"]
    +        )
     
             identity = await self.adapter._resolve_room_identity("!stale:ex.org")
     
             assert identity.chat_type == "room"
             assert identity.conflict is True
    +        assert identity.joined_member_count == 3
             assert await self.adapter._is_dm_room("!stale:ex.org") is False
     
         @pytest.mark.asyncio
    @@ -2057,7 +2088,7 @@ def handle_sync(sync_data):
             fake_client.join_room.assert_awaited_once()
             assert "!room:example.org" in adapter._joined_rooms
             assert len(captured) == 1
    -        assert captured[0].source.chat_type == "group"
    +        assert captured[0].source.chat_type == "dm"
     
         @pytest.mark.asyncio
         async def test_seconds_timestamp_is_not_treated_as_milliseconds(self):

  • tests/gateway/test_matrix_project_context_isolation.py

    @@ -168,6 +168,9 @@ async def test_matrix_project_context_survives_sequential_messages():
     @pytest.mark.asyncio
     async def test_matrix_session_scope_auto_and_thread_preserve_synthetic_threads():
         adapter = _make_adapter()
    +    # Override member_count to 3 so the named project room is NOT classified as
    +    # a DM (the DM fix uses member_count <= 2 as the primary DM signal).
    +    adapter._get_room_member_count = AsyncMock(return_value=3)
         adapter._auto_thread = True
         adapter._matrix_session_scope = "auto"
         auto_source = await _source_for(adapter, PROJECT_B_ROOM_ID, "$auto")

• #54129 fix(matrix): record DM rooms in m.direct on invite (#44679)

  merged to main

  files: plugins/platforms/matrix/adapter.py, tests/gateway/test_matrix_dm_invite_recording.py

  Show the diff

  • plugins/platforms/matrix/adapter.py

    @@ -2885,15 +2885,27 @@ async def _handle_media_message(
             await self.handle_message(msg_event)
     
         async def _on_invite(self, event: Any) -> None:
    -        """Auto-join rooms when invited."""
    +        """Auto-join rooms when invited, recording DM rooms in m.direct."""
     
             room_id = str(getattr(event, "room_id", ""))
    +        content = getattr(event, "content", None)
    +        is_direct = bool(getattr(content, "is_direct", False))
    +        inviter = str(getattr(event, "sender", ""))
     
             logger.info(
    -            "Matrix: invited to %s — joining",
    +            "Matrix: invited to %s — joining (is_direct=%s)",
    +            room_id,
    +            is_direct,
    +        )
    +        # When the invite declares this as a DM, record it in m.direct after
    +        # the (non-blocking) join completes so that _resolve_room_identity
    +        # treats it correctly even when the bot account has no prior DM
    +        # history. The join itself stays off the sync path.
    +        self._schedule_invite_join(
                 room_id,
    +            is_direct=is_direct and bool(inviter),
    +            inviter=inviter,
             )
    -        self._schedule_invite_join(room_id)
     
         async def _join_room_by_id(self, room_id: str) -> bool:
             """Join a room by ID and refresh local caches on success."""
    @@ -2913,7 +2925,13 @@ async def _join_room_by_id(self, room_id: str) -> bool:
                 logger.warning("Matrix: error joining %s: %s", room_id, exc)
                 return False
     
    -    def _schedule_invite_join(self, room_id: str) -> None:
    +    def _schedule_invite_join(
    +        self,
    +        room_id: str,
    +        *,
    +        is_direct: bool = False,
    +        inviter: str = "",
    +    ) -> None:
             """Schedule an invite join without blocking sync or gateway readiness."""
             if not room_id or room_id in self._joined_rooms:
                 return
    @@ -2923,7 +2941,13 @@ def _schedule_invite_join(self, room_id: str) -> None:
     
             async def _join_invite() -> None:
                 try:
    -                await asyncio.wait_for(self._join_room_by_id(room_id), timeout=45.0)
    +                joined = await asyncio.wait_for(
    +                    self._join_room_by_id(room_id), timeout=45.0
    +                )
    +                # Persist the DM signal from the invite once the join lands,
    +                # so m.direct is authoritative even on a fresh bot account.
    +                if joined and is_direct and inviter:
    +                    await self._record_dm_room(room_id, inviter)
                 except asyncio.TimeoutError:
                     logger.warning("Matrix: timed out joining invite %s", room_id)
                 finally:
    @@ -3779,6 +3803,47 @@ async def _refresh_dm_cache(self) -> None:
             self._room_identities.clear()
             self._room_identity_cached_at.clear()
     
    +    async def _record_dm_room(self, room_id: str, inviter: str) -> None:
    +        """Persist a room as DM in m.direct account data after an invite.
    +
    +        When the bot account has never been used for DMs, ``m.direct`` is
    +        absent (404).  This method fetches the current mapping (if any),
    +        appends *room_id* under the *inviter*'s entry, and writes it back
    +        so that subsequent ``_refresh_dm_cache`` calls treat the room as a
    +        DM without requiring manual ``m.direct`` setup.
    +        """
    +        if not self._client:
    +            return
    +
    +        dm_data: Dict[str, list] = {}
    +        try:
    +            resp = await self._client.get_account_data("m.direct")
    +            if hasattr(resp, "content") and isinstance(resp.content, dict):
    +                dm_data = resp.content
    +            elif isinstance(resp, dict):
    +                dm_data = resp
    +        except Exception:
    +            pass  # m.direct doesn't exist yet — start fresh
    +
    +        rooms_for_user = dm_data.get(inviter, [])
    +        if not isinstance(rooms_for_user, list):
    +            rooms_for_user = []
    +        if room_id not in rooms_for_user:
    +            rooms_for_user.append(room_id)
    +            dm_data[inviter] = rooms_for_user
    +            try:
    +                await self._client.set_account_data("m.direct", dm_data)
    +                logger.info(
    +                    "Matrix: recorded %s as DM room (inviter=%s)", room_id, inviter
    +                )
    +            except Exception as exc:
    +                logger.warning("Matrix: failed to update m.direct: %s", exc)
    +
    +        # Update local cache so _resolve_room_identity sees it immediately.
    +        self._dm_rooms[room_id] = True
    +        self._room_identities.pop(room_id, None)
    +        self._room_identity_cached_at.pop(room_id, None)
    +
         # ------------------------------------------------------------------
         # Mention detection helpers
         # ------------------------------------------------------------------

  • tests/gateway/test_matrix_dm_invite_recording.py

    @@ -0,0 +1,255 @@
    +"""Tests for Matrix DM room recording on invite (issue #44679).
    +
    +When the bot's Matrix account has no ``m.direct`` account data (common for
    +accounts created solely for Hermes), DM rooms are silently treated as groups.
    +This tests the fix that records DM rooms in ``m.direct`` when the invite
    +event carries ``is_direct: true``.
    +"""
    +
    +import time
    +from types import SimpleNamespace
    +from unittest.mock import AsyncMock, MagicMock, patch
    +
    +import pytest
    +
    +from gateway.config import PlatformConfig
    +
    +
    +def _make_adapter(tmp_path=None):
    +    """Create a MatrixAdapter with mocked config."""
    +    from plugins.platforms.matrix.adapter import MatrixAdapter
    +
    +    config = PlatformConfig(
    +        enabled=True,
    +        token="[redacted:secret-assignment]",
    +        extra={
    +            "homeserver": "https://matrix.example.org",
    +            "user_id": "@hermes:example.org",
    +        },
    +    )
    +    adapter = MatrixAdapter(config)
    +    adapter._text_batch_delay_seconds = 0
    +    adapter.handle_message = AsyncMock()
    +    adapter._startup_ts = time.time() - 10
    +    return adapter
    +
    +
    +def _make_invite_event(
    +    room_id="!dm_room:example.org",
    +    sender="@alice:example.org",
    +    is_direct=True,
    +):
    +    """Create a fake invite event with is_direct in content."""
    +    content = SimpleNamespace(is_direct=is_direct)
    +    return SimpleNamespace(
    +        room_id=room_id,
    +        sender=sender,
    +        content=content,
    +    )
    +
    +
    +# ---------------------------------------------------------------------------
    +# _on_invite DM recording
    +# ---------------------------------------------------------------------------
    +
    +
    +class TestOnInviteRecordsDM:
    +    """_on_invite schedules a join that records the DM when is_direct is True.
    +
    +    The join itself is non-blocking (``_schedule_invite_join`` spawns a task),
    +    so these tests drive ``_on_invite`` and then await the scheduled task to
    +    observe its side effects.
    +    """
    +
    +    @staticmethod
    +    async def _drain_invite_tasks(adapter):
    +        """Await any tasks _schedule_invite_join spawned."""
    +        tasks = list(adapter._invite_join_tasks.values())
    +        for task in tasks:
    +            await task
    +
    +    @pytest.mark.asyncio
    +    async def test_dm_invite_records_room(self):
    +        adapter = _make_adapter()
    +        adapter._join_room_by_id = AsyncMock(return_value=True)
    +        adapter._record_dm_room = AsyncMock()
    +
    +        event = _make_invite_event(is_direct=True, sender="@alice:example.org")
    +        await adapter._on_invite(event)
    +        await self._drain_invite_tasks(adapter)
    +
    +        adapter._join_room_by_id.assert_awaited_once_with("!dm_room:example.org")
    +        adapter._record_dm_room.assert_awaited_once_with(
    +            "!dm_room:example.org", "@alice:example.org"
    +        )
    +
    +    @pytest.mark.asyncio
    +    async def test_non_dm_invite_does_not_record(self):
    +        adapter = _make_adapter()
    +        adapter._join_room_by_id = AsyncMock(return_value=True)
    +        adapter._record_dm_room = AsyncMock()
    +
    +        event = _make_invite_event(is_direct=False)
    +        await adapter._on_invite(event)
    +        await self._drain_invite_tasks(adapter)
    +
    +        adapter._join_room_by_id.assert_awaited_once()
    +        adapter._record_dm_room.assert_not_awaited()
    +
    +    @pytest.mark.asyncio
    +    async def test_missing_is_direct_does_not_record(self):
    +        """Invite events without is_direct attribute should not trigger recording."""
    +        adapter = _make_adapter()
    +        adapter._join_room_by_id = AsyncMock(return_value=True)
    +        adapter._record_dm_room = AsyncMock()
    +
    +        event = SimpleNamespace(
    +            room_id="!room:example.org",
    +            sender="@alice:example.org",
    +            content=SimpleNamespace(),  # no is_direct attr
    +        )
    +        await adapter._on_invite(event)
    +        await self._drain_invite_tasks(adapter)
    +
    +        adapter._record_dm_room.assert_not_awaited()
    +
    +    @pytest.mark.asyncio
    +    async def test_join_failure_does_not_record(self):
    +        adapter = _make_adapter()
    +        adapter._join_room_by_id = AsyncMock(return_value=False)
    +        adapter._record_dm_room = AsyncMock()
    +
    +        event = _make_invite_event(is_direct=True)
    +        await adapter._on_invite(event)
    +        await self._drain_invite_tasks(adapter)
    +
    +        adapter._record_dm_room.assert_not_awaited()
    +
    +    @pytest.mark.asyncio
    +    async def test_empty_inviter_does_not_record(self):
    +        adapter = _make_adapter()
    +        adapter._join_room_by_id = AsyncMock(return_value=True)
    +        adapter._record_dm_room = AsyncMock()
    +
    +        event = SimpleNamespace(
    +            room_id="!room:example.org",
    +            sender="",
    +            content=SimpleNamespace(is_direct=True),
    +        )
    +        await adapter._on_invite(event)
    +        await self._drain_invite_tasks(adapter)
    +
    +        adapter._record_dm_room.assert_not_awaited()
    +
    +
    +# ---------------------------------------------------------------------------
    +# _record_dm_room
    +# ---------------------------------------------------------------------------
    +
    +
    +class TestRecordDMRoom:
    +    """_record_dm_room should update m.direct account data and local cache."""
    +
    +    @pytest.mark.asyncio
    +    async def test_creates_m_direct_when_absent(self):
    +        """When m.direct doesn't exist (404), creates it from scratch."""
    +        adapter = _make_adapter()
    +        adapter._client = MagicMock()
    +        adapter._client.get_account_data = AsyncMock(side_effect=Exception("M_NOT_FOUND"))
    +        adapter._client.set_account_data = AsyncMock()
    +
    +        await adapter._record_dm_room("!new:example.org", "@alice:example.org")
    +
    +        adapter._client.set_account_data.assert_awaited_once_with(
    +            "m.direct", {"@alice:example.org": ["!new:example.org"]}
    +        )
    +        assert adapter._dm_rooms.get("!new:example.org") is True
    +
    +    @pytest.mark.asyncio
    +    async def test_appends_to_existing_m_direct(self):
    +        """When m.direct exists with other rooms, appends the new room."""
    +        adapter = _make_adapter()
    +        adapter._client = MagicMock()
    +        existing_data = {"@bob:example.org": ["!old:example.org"]}
    +        adapter._client.get_account_data = AsyncMock(return_value=existing_data)
    +        adapter._client.set_account_data = AsyncMock()
    +
    +        await adapter._record_dm_room("!new:example.org", "@alice:example.org")
    +
    +        expected = {
    +            "@bob:example.org": ["!old:example.org"],
    +            "@alice:example.org": ["!new:example.org"],
    +        }
    +        adapter._client.set_account_data.assert_awaited_once_with("m.direct", expected)
    +
    +    @pytest.mark.asyncio
    +    async def test_no_duplicate_room_in_m_direct(self):
    +        """If room is already in m.direct, does not append again."""
    +        adapter = _make_adapter()
    +        adapter._client = MagicMock()
    +        existing_data = {"@alice:example.org": ["!room:example.org"]}
    +        adapter._client.get_account_data = AsyncMock(return_value=existing_data)
    +        adapter._client.set_account_data = AsyncMock()
    +
    +        await adapter._record_dm_room("!room:example.org", "@alice:example.org")
    +
    +        adapter._client.set_account_data.assert_not_awaited()
    +        assert adapter._dm_rooms.get("!room:example.org") is True
    +
    +    @pytest.mark.asyncio
    +    async def test_set_failure_is_handled_gracefully(self):
    +        """If set_account_data fails, local cache is still updated."""
    +        adapter = _make_adapter()
    +        adapter._client = MagicMock()
    +        adapter._client.get_account_data = AsyncMock(side_effect=Exception("not found"))
    +        adapter._client.set_account_data = AsyncMock(
    +            side_effect=Exception("M_FORBIDDEN")
    +        )
    +
    +        # Should not raise
    +        await adapter._record_dm_room("!room:example.org", "@alice:example.org")
    +
    +        # Local cache updated despite server error
    +        assert adapter._dm_rooms.get("!room:example.org") is True
    +
    +    @pytest.mark.asyncio
    +    async def test_clears_room_identity_cache(self):
    +        """After recording a DM, room identity cache should be invalidated."""
    +        adapter = _make_adapter()
    +        adapter._client = MagicMock()
    +        adapter._client.get_account_data = AsyncMock(side_effect=Exception("404"))
    +        adapter._client.set_account_data = AsyncMock()
    +
    +        adapter._room_identities["!room:example.org"] = "stale"
    +        adapter._room_identity_cached_at["!room:example.org"] = time.monotonic()
    +
    +        await adapter._record_dm_room("!room:example.org", "@alice:example.org")
    +
    +        assert "!room:example.org" not in adapter._room_identities
    +        assert "!room:example.org" not in adapter._room_identity_cached_at
    +
    +    @pytest.mark.asyncio
    +    async def test_no_client_is_noop(self):
    +        """If _client is None, does nothing."""
    +        adapter = _make_adapter()
    +        adapter._client = None
    +
    +        # Should not raise
    +        await adapter._record_dm_room("!room:example.org", "@alice:example.org")
    +
    +    @pytest.mark.asyncio
    +    async def test_m_direct_response_with_content_attr(self):
    +        """get_account_data may return an object with .content attribute."""
    +        adapter = _make_adapter()
    +        adapter._client = MagicMock()
    +        resp = SimpleNamespace(content={"@bob:example.org": ["!old:example.org"]})
    +        adapter._client.get_account_data = AsyncMock(return_value=resp)
    +        adapter._client.set_account_data = AsyncMock()
    +
    +        await adapter._record_dm_room("!new:example.org", "@alice:example.org")
    +
    +        expected = {
    +            "@bob:example.org": ["!old:example.org"],
    +            "@alice:example.org": ["!new:example.org"],
    +        }
    +        adapter._client.set_account_data.assert_awaited_once_with("m.direct", expected)