Browse Source

Merge branch 'review-baseadapter-overrides' into main

Chat-type mapping for the gateway's session vocabulary, plus the note on what
group vs channel actually changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul-Dieter Klumpp 1 week ago
parent
commit
0c2f174846
2 changed files with 150 additions and 3 deletions
  1. 95 3
      adapter.py
  2. 55 0
      test_adapter.py

+ 95 - 3
adapter.py

@@ -33,6 +33,7 @@ import logging
 import mimetypes
 import os
 from datetime import datetime, timezone
+from enum import StrEnum
 from typing import Any, Dict, List, Literal, Optional, Tuple, cast
 from urllib.parse import urlsplit
 
@@ -96,6 +97,98 @@ except ImportError:  # pragma: no cover - loaded as a top-level module (tests)
     )
 
 
+# --------------------------------------------------------------------------- #
+# Chat types
+# --------------------------------------------------------------------------- #
+
+class HermesChatType(StrEnum):
+    """The ``chat_type`` vocabulary the Hermes gateway understands.
+
+    Declared in ``gateway/session.py:161`` as ``"dm", "group", "channel",
+    "thread"`` and consumed as a bare string all over the gateway:
+    ``SessionSource.description`` (session.py:239) and the PII-redacting
+    description in ``build_session_context_prompt`` (session.py:537) both
+    branch on these exact values and fall back to a nameless generic case for
+    anything else, and ``build_session_key`` puts the value straight into the
+    session key.  Passing a chattolib ``RoomKind`` (``"ROOM_KIND_CHANNEL"``)
+    therefore does not fail loudly — it just quietly degrades what the agent is
+    told about where it is.
+
+    A StrEnum so it stays a drop-in ``str`` at every one of those call sites.
+
+    GROUP vs CHANNEL
+    ----------------
+    There is no strict contract between the two, and the adapters disagree in
+    practice: Slack labels every non-DM conversation ``"group"`` (including real
+    channels), Discord uses both, and Telegram reserves ``"channel"`` for actual
+    broadcast channels.  The intended reading is ``group`` = ordinary
+    multi-participant chat, ``channel`` = broadcast surface.
+
+    The distinction only changes behaviour in three places:
+
+    1. Authorization (``gateway/authz_mixin.py``) — the only security-relevant
+       one.  The group-scoped env allowlists apply to ``{"group", "forum"}``
+       ONLY, never to ``"channel"``: ``{PLATFORM}_GROUP_ALLOWED_USERS`` /
+       ``_GROUP_ALLOWED_CHATS`` (:616), the chat-id allowlist (:708) and the
+       Telegram legacy shim (:724).  The adapter-delegation paths in turn treat
+       all three alike (:461, :649, :674, :694), where the value only picks
+       ``group_allow_from`` over ``allow_from`` from ``config.extra``.
+       For Chatto both choices are equivalent today: those group env maps hold
+       Telegram and QQBot only (:535-541), and our own allowlist runs through
+       ``CHATTO_ALLOWED_USERS``, which is chat_type-independent.
+    2. What the agent is told — ``SessionSource.description`` renders
+       ``"group: Name"`` vs ``"channel: Name"`` (session.py:239-246), likewise
+       the PII-redacted variant (session.py:537-544).
+    3. The session key, which embeds the literal (session.py:1192).  Changing
+       the value for a room re-buckets its existing sessions.
+
+    Explicitly NOT affected: ``is_shared_multi_user_session`` (session.py:1063)
+    only looks at ``"dm"`` and ``thread_id``, so sender prefixes, the multi-user
+    prompt line and ``group_sessions_per_user`` treat group and channel
+    identically.
+    """
+
+    DM = "dm"
+    GROUP = "group"
+    CHANNEL = "channel"
+    # Emitted by adapters whose thread events are their own chat type (Slack,
+    # Discord). We don't: a Chatto thread keeps its room's chat_type and is
+    # identified by ``thread_id`` on the source instead. Listed for the record,
+    # because build_session_key rewrites the slot to "thread" itself
+    # (session.py:1190).
+    THREAD = "thread"
+    # Not declared in session.py:161 but real: Telegram forum topics travel as
+    # "forum", and the authz group allowlists above accept it alongside "group".
+    # Chatto has no equivalent, so we never emit it.
+
+
+# Chatto only distinguishes DMs from channels. UNSPECIFIED means the server
+# sent a kind this vendored chattolib doesn't know: map it to the generic
+# multi-user bucket rather than guessing "channel", and never to "dm" — that
+# value drives session isolation (is_shared_multi_user_session, session.py:1063)
+# and would silently turn a room into a private conversation.
+#
+# CHANNEL for RoomKind.CHANNEL is the descriptive choice and carries no
+# behavioural cost (see the GROUP vs CHANNEL note above). Switching to GROUP for
+# Slack parity would be this one line — plus the re-bucketing of existing
+# sessions that point 3 of that note describes.
+_ROOM_KIND_TO_CHAT_TYPE: Dict[RoomKind, HermesChatType] = {
+    RoomKind.DM: HermesChatType.DM,
+    RoomKind.CHANNEL: HermesChatType.CHANNEL,
+    RoomKind.UNSPECIFIED: HermesChatType.GROUP,
+}
+
+
+def chat_type_for_room_kind(kind: Optional[RoomKind]) -> HermesChatType:
+    """Map a chattolib RoomKind onto the gateway's chat_type vocabulary.
+
+    An unknown or missing kind becomes ``GROUP`` — see ``_ROOM_KIND_TO_CHAT_TYPE``.
+    """
+    if kind is None:
+        return HermesChatType.GROUP
+    return _ROOM_KIND_TO_CHAT_TYPE.get(kind, HermesChatType.GROUP)
+
+
 # --------------------------------------------------------------------------- #
 # Adapter
 # --------------------------------------------------------------------------- #
@@ -573,7 +666,7 @@ class ChattoAdapter(BasePlatformAdapter):
         source = self.build_source(
             chat_id=payload.room_id,
             chat_name=self._room_names.get(message.room_id),
-            chat_type="dm" if room_kind == RoomKind.DM else room_kind or RoomKind.UNSPECIFIED, # Only "dm" seems to be a reserved keyword from Base adapter class.,
+            chat_type=chat_type_for_room_kind(room_kind),
             user_id=message.actor_id,
             user_name=user.login, # use login, because display_name is changeable by anyone.
             thread_id=thread_id,
@@ -1158,10 +1251,9 @@ class ChattoAdapter(BasePlatformAdapter):
         """
         name = self._room_names.get(chat_id, chat_id)
         kind = self._room_kinds.get(chat_id)
-        chat_type = "dm" if kind == RoomKind.DM else "group"
         return {
             "name": name,
-            "type": chat_type,
+            "type": chat_type_for_room_kind(kind).value,
         }
 
     # ------------------------------------------------------------------ #

+ 55 - 0
test_adapter.py

@@ -34,6 +34,8 @@ sys.path.insert(0, "/root/.hermes/plugins/platforms/chatto")
 
 from adapter import (
     ChattoAdapter,
+    HermesChatType,
+    chat_type_for_room_kind,
     hermes_check_fn as check_requirements,
     hermes_validate_config as validate_config,
     register,
@@ -658,6 +660,59 @@ class TestReactionForwarding:
         await adapter._handle_realtime_event(self._event("reaction_added"))
 
 
+# -- chat_type mapping --
+
+class TestChatTypeMapping:
+    """RoomKind -> the gateway's chat_type vocabulary."""
+
+    def test_maps_known_kinds(self):
+        assert chat_type_for_room_kind(RoomKind.DM) is HermesChatType.DM
+        assert chat_type_for_room_kind(RoomKind.CHANNEL) is HermesChatType.CHANNEL
+
+    def test_unknown_kind_is_group_never_dm(self):
+        """'dm' drives session isolation — never guess it for an unknown kind."""
+        assert chat_type_for_room_kind(RoomKind.UNSPECIFIED) is HermesChatType.GROUP
+        assert chat_type_for_room_kind(None) is HermesChatType.GROUP
+
+    def test_values_match_the_gateway_vocabulary(self):
+        """session.py:161 declares exactly these strings; SessionSource.description
+        and the PII-redacted context prompt branch on them."""
+        assert [t.value for t in HermesChatType] == ["dm", "group", "channel", "thread"]
+
+    def test_is_a_plain_str_at_call_sites(self):
+        assert HermesChatType.CHANNEL == "channel"
+        assert f"{HermesChatType.DM}" == "dm"
+
+    async def test_get_chat_info_reports_channel(self):
+        adapter = _make_adapter()
+        adapter._room_names["room-1"] = "Team"
+        adapter._room_kinds["room-1"] = RoomKind.CHANNEL
+        info = await adapter.get_chat_info("room-1")
+        assert info == {"name": "Team", "type": "channel"}
+
+    async def test_get_chat_info_reports_dm(self):
+        adapter = _make_adapter()
+        adapter._room_kinds["dm-1"] = RoomKind.DM
+        assert (await adapter.get_chat_info("dm-1"))["type"] == "dm"
+
+    async def test_dispatch_stamps_the_mapped_chat_type(self):
+        """The value reaching build_source decides how the agent is told where
+        it is — a raw RoomKind lands in SessionSource.description's else-branch."""
+        adapter = _make_adapter()
+        adapter.chatto_config.allow_all_users.value = True
+        adapter.me = _make_user("bot-user-id", "hermes_bot")
+        adapter._room_kinds["room-1"] = RoomKind.CHANNEL
+        adapter._user_cache["user-1"] = _make_user("user-1", "alice")
+        adapter.handle_message = AsyncMock()
+        payload = _make_posted_payload()
+        payload.fetch_message = AsyncMock(return_value=_make_message(body="hi"))
+
+        await adapter._dispatch_message_posted(payload)
+
+        event = adapter.handle_message.call_args.args[0]
+        assert event.source.chat_type == "channel"
+
+
 # -- Inbound attachments --
 
 class TestInboundAttachments: