Review cleanup: typed access, dead state, markers, honest docs

From the adapter review:

- Typed access: read Asset.id, MessageAttachment fields, Message.id and
  ChattoRealtimeError.fatal directly instead of getattr-with-default
  (the exact pattern AGENTS.md forbids after the upload-id incident);
  _cache_attachments takes list[MessageAttachment] and drops its unused
  room_id parameter; the edit dispatcher reads pending.message_id.
- Dead state: _our_message_ids/_our_thread_roots were write-only since
  their feature never existed - deleted with all five add() sites.
- connect()'s assert becomes a plain guard; hermes_validate_config
  loses its unreachable base_url branch (base_url always resolves to
  the ChattoHQ default).
- Override markers added to add_reaction/remove_reaction/start_dm/
  create_room.
- Docs say what the code does: _check_auth explains why it exists,
  hermes_is_connected admits it only validates configuration, the
  dismiss-all call runs once instead of per room, register() stops
  logging an env var name, local imports move to the top.

adapter.py

@@ -28,10 +28,13 @@ import hashlib
 import logging
 import mimetypes
 import os
+import tempfile
 from datetime import UTC, datetime
 from enum import StrEnum
 from typing import Any, cast
-from urllib.parse import urlsplit
+from urllib.parse import unquote, urlsplit
+
+import httpx
 
 logger = logging.getLogger(__name__)
 
@@ -77,6 +80,7 @@ try:
     )
     from chattolib.types import (
         Message,
+        MessageAttachment,
         PresenceStatus,
         Room,
         RoomKind,
@@ -245,7 +249,7 @@ class ChattoAdapter(BasePlatformAdapter):
         super().__init__(
             config=pconfig, platform=Platform(ChattoConstants.PLATFORM_NAME)
         )
-        # "extra" has been pre-processed by Hermes-Framework to be a dict of extra config values insode PlatformConfig.
+        # "extra" has been pre-populated by Hermes from config.yaml's extra block.
 
         # --- Configuration from our configuration data class with some logic ---
         self.chatto_config: ChattoConfiguration = ChattoConfiguration(pconfig)
@@ -258,10 +262,6 @@ class ChattoAdapter(BasePlatformAdapter):
         # --- Runtime state ---
         self._room_names: dict[str, str] = {}
         self._room_kinds: dict[str, RoomKind] = {}
-        self._our_thread_roots: set = set()  # thread root event IDs we created
-        self._our_message_ids: set = (
-            set()
-        )  # message IDs we sent (for thread root detection)
         # Event IDs already processed — chattolib may redeliver events across
         # reconnects, so every inbound event is checked against this list.
         self._seen: list[str] = []
@@ -419,8 +419,8 @@ class ChattoAdapter(BasePlatformAdapter):
                 retryable=False,
             )
             try:
-                assert self._chatto_client
-                await self._chatto_client.close()
+                if self._chatto_client is not None:
+                    await self._chatto_client.close()
             finally:
                 self._chatto_client = None
             return False
@@ -628,9 +628,14 @@ class ChattoAdapter(BasePlatformAdapter):
         return False
 
     def _check_auth(self, user: User) -> bool:
-        """We roll our own auth-systen on the against the chatto_config'ured allowed_users etc.
-
-        because.. Hermes authz_mixin.py IS NOT SANE.
+        """Whether this Chatto user may talk to the agent.
+
+        Deliberately our own gate instead of the gateway's authz_mixin: its
+        group allowlists key on chat_type and per-platform env vars
+        (``{PLATFORM}_GROUP_ALLOWED_USERS``), none of which fit Chatto's one
+        flat member directory. ``CHATTO_ALLOWED_USERS`` matches login and id,
+        ``CHATTO_ALLOW_ALL_USERS`` overrides both — the gate stays
+        chat_type-independent by design.
         """
         if self.chatto_config.allow_all_users.value:
             return True
@@ -797,8 +802,6 @@ class ChattoAdapter(BasePlatformAdapter):
         as chunks arrive, because a missing or lying header must not smuggle an
         unbounded body past the cap.
         """
-        import httpx
-
         max_bytes = get_inbound_media_max_bytes()
         chunks: list[bytes] = []
         total = 0
@@ -834,8 +837,7 @@ class ChattoAdapter(BasePlatformAdapter):
 
     async def _cache_attachments(
         self,
-        room_id: str,
-        attachments: list[Any],
+        attachments: list[MessageAttachment],
     ) -> tuple[list[str], list[str], list[str]]:
         """Download message attachments into the gateway media cache.
 
@@ -848,11 +850,10 @@ class ChattoAdapter(BasePlatformAdapter):
         media_types: list[str] = []
         media_kinds: list[str] = []
 
-        for att in attachments or []:
-            asset_url = getattr(att, "asset_url", None)
-            url = getattr(asset_url, "url", "") if asset_url else ""
-            filename = getattr(att, "filename", "") or ""
-            content_type = getattr(att, "content_type", "") or ""
+        for att in attachments:
+            url = att.asset_url.url if att.asset_url else ""
+            filename = att.filename or ""
+            content_type = att.content_type or ""
             if not url:
                 # Videos are announced before transcoding finishes, so the
                 # signed URL can legitimately be missing on arrival.
@@ -1045,7 +1046,7 @@ class ChattoAdapter(BasePlatformAdapter):
             was_dispatched
             and payload.message_event_id not in self._processing.values()
             and not any(
-                getattr(pending, "message_id", None) == payload.message_event_id
+                pending.message_id == payload.message_event_id
                 for pending in self._pending_messages.values()
             )
         ):
@@ -1244,15 +1245,11 @@ class ChattoAdapter(BasePlatformAdapter):
 
         # Attachments — download and hand the local cache paths to the gateway,
         # which runs vision enrichment / document extraction off media_urls.
-        attachments = list(message.attachments or [])
         (
             message_event.media_urls,
             message_event.media_types,
             media_kinds,
-        ) = await self._cache_attachments(
-            room_id,
-            attachments,
-        )
+        ) = await self._cache_attachments(list(message.attachments))
         if media_kinds:
             # Same precedence as the Teams/Signal adapters: document-context
             # injection gates strictly on DOCUMENT, image handling keys off the
@@ -1416,7 +1413,7 @@ class ChattoAdapter(BasePlatformAdapter):
                 await self._sleep_interruptible(wait)
                 continue
             except ChattoRealtimeError as exc:
-                if getattr(exc, "fatal", False):
+                if exc.fatal:
                     logger.error(
                         "Chatto: fatal realtime error (%s): %s", exc.code, exc.message
                     )
@@ -1579,11 +1576,15 @@ class ChattoAdapter(BasePlatformAdapter):
         for _rid in list(self._joined_room_ids):
             try:
                 await client.mark_room_as_read(room_id=_rid)
-                await client.dismiss_all_notifications()
             except Exception:
                 logger.debug(
                     "Chatto: mark_room_as_read failed for %s", _rid, exc_info=True
                 )
+        # Dismissal is server-global — once after the per-room sweep.
+        try:
+            await client.dismiss_all_notifications()
+        except Exception:
+            logger.debug("Chatto: dismiss_all_notifications failed", exc_info=True)
 
     # ------------------------------------------------------------------ #
     # Sending (ConnectRPC — unchanged from polling version)
@@ -1667,11 +1668,6 @@ class ChattoAdapter(BasePlatformAdapter):
 
             self._mark_seen(msg_obj.id)
             message_ids.append(msg_obj.id)
-            self._our_message_ids.add(msg_obj.id)
-            # If we sent a message WITHOUT a thread_id, this message could
-            # become a thread root if someone replies to it
-            if not thread_id:
-                self._our_thread_roots.add(msg_obj.id)
             # Auto-thread: first chunk becomes the thread root,
             # subsequent chunks go in the thread
             if use_auto_thread and i == 0 and not thread_id:
@@ -1796,7 +1792,7 @@ class ChattoAdapter(BasePlatformAdapter):
 
         # Our own edit comes back as a message_edited event; mark it seen so it
         # is never mistaken for inbound traffic.
-        edited_id = getattr(msg, "id", "") or str(message_id)
+        edited_id = msg.id or str(message_id)
         self._mark_seen(edited_id)
         return SendResult(success=True, message_id=edited_id)
 
@@ -1865,14 +1861,12 @@ class ChattoAdapter(BasePlatformAdapter):
             )
             return None
 
-        seed_id = getattr(msg, "id", "") or ""
+        seed_id = msg.id
         if not seed_id:
             logger.warning("Chatto: handoff thread seed-post returned no message id")
             return None
 
         self._mark_seen(seed_id)
-        self._our_message_ids.add(seed_id)
-        self._our_thread_roots.add(seed_id)
         try:
             await client.follow_thread(str(parent_chat_id), seed_id)
         except Exception:
@@ -1964,7 +1958,10 @@ class ChattoAdapter(BasePlatformAdapter):
         return emoji
 
     async def add_reaction(self, room_id: str, message_id: str, emoji: str) -> bool:
-        """Add a reaction to a message via MessageService/AddReaction."""
+        """Add a reaction to a message via MessageService/AddReaction.
+
+        BasePlatformAdapter override
+        """
         shortcode = self._emoji_to_shortcode(emoji)
         client = await self._get_chatto_client()
         if client is None:
@@ -1985,7 +1982,10 @@ class ChattoAdapter(BasePlatformAdapter):
             return False
 
     async def remove_reaction(self, chat_id: str, message_id: str, emoji: str) -> bool:
-        """Remove a reaction from a message via MessageService/RemoveReaction."""
+        """Remove a reaction from a message via MessageService/RemoveReaction.
+
+        BasePlatformAdapter override
+        """
         shortcode = self._emoji_to_shortcode(emoji)
         client = await self._get_chatto_client()
         if client is None:
@@ -2013,6 +2013,8 @@ class ChattoAdapter(BasePlatformAdapter):
         """Start a direct message with a user via RoomService/StartDM.
 
         Returns the room ID on success, or None on failure.
+
+        BasePlatformAdapter override
         """
         if not user_id:
             return None
@@ -2045,6 +2047,8 @@ class ChattoAdapter(BasePlatformAdapter):
         """Create an ad-hoc room via RoomService/CreateRoom.
 
         Returns the room ID on success, or None on failure.
+
+        BasePlatformAdapter override
         """
         client = await self._get_chatto_client()
         if client is None:
@@ -2208,7 +2212,7 @@ class ChattoAdapter(BasePlatformAdapter):
                 logger.error("Chatto: CompleteUpload returned no asset")
                 return None
 
-            asset_id = str(getattr(cast(Any, asset), "id", ""))
+            asset_id = asset.id
             logger.info(
                 "Chatto: uploaded %s as asset %s (%d bytes)",
                 file_name,
@@ -2250,7 +2254,6 @@ class ChattoAdapter(BasePlatformAdapter):
                 thread_root_event_id=str(thread_id) if thread_id else "",
             )
             self._mark_seen(msg.id)
-            self._our_message_ids.add(msg.id)
             return SendResult(success=True, message_id=msg.id)
         except ChattoError as e:
             return SendResult(success=False, error=str(e), retryable=True)
@@ -2453,11 +2456,8 @@ class ChattoAdapter(BasePlatformAdapter):
 
         Accepts ``http(s)://`` URLs (downloaded to a temp file), ``file://``
         URIs and bare paths.  Returns ``(path, is_temp)`` — the caller unlinks
-        when ``is_temp``.  ``(None, False)`` means the entry is unusable.
+        when ``is_temp``.          ``(None, False)`` means the entry is unusable.
         """
-        import tempfile
-        from urllib.parse import unquote as _unquote
-
         if image_url.startswith(("http://", "https://")):
             parsed = urlsplit(image_url)
             ext = os.path.splitext(parsed.path)[1] or ".png"
@@ -2477,7 +2477,7 @@ class ChattoAdapter(BasePlatformAdapter):
 
         local = image_url
         if local.startswith("file://"):
-            local = _unquote(urlsplit(local).path)
+            local = unquote(urlsplit(local).path)
         return self.validate_media_delivery_path(local), False
 
     async def send_multiple_images(
@@ -2645,18 +2645,13 @@ def hermes_validate_config(config: PlatformConfig) -> bool:
         )
         return False
 
-    if chatto_config.base_url.value:
-        if (chatto_config.token.value is not None) or (
-            chatto_config.login.value and chatto_config.password.value
-        ):
-            return True
-        else:
-            logger.error(
-                "Chatto: Minimally, either token or login/password must be set."
-            )
-    else:
-        logger.error("Chatto: base_url must be set.")
-
+    # base_url always resolves (it defaults to ChattoHQ), so the only real
+    # question is whether any credentials came in.
+    if (chatto_config.token.value is not None) or (
+        chatto_config.login.value and chatto_config.password.value
+    ):
+        return True
+    logger.error("Chatto: Minimally, either token or login/password must be set.")
     return False
 
 
@@ -2678,9 +2673,12 @@ def hermes_check_fn() -> bool:
 
 
 def hermes_is_connected(config: PlatformConfig) -> bool:
-    """Check whether Chatto Plugin is connected. But where to: the Hermes Agent or the Chatto server.
+    """Report whether the Chatto platform is configured and enabled.
 
-    The Hermes Agent plugin docs suck and it seems there are many functions to do the same.
+    The name is fixed by the register() contract, but despite what it
+    suggests this does not open a connection — it validates configuration
+    only (see :func:`hermes_validate_config`); the gateway probes real
+    connectivity through ``connect()``.
     """
     return bool(hermes_validate_config(config) and config.enabled)
 
@@ -2814,11 +2812,6 @@ def register(ctx) -> None:
     for capability in _capabilities():
         logger.info("Chatto capability: %s", capability)
 
-    logger.info(
-        "ChattoConfiguration.allowed_users.env_name: %s",
-        ChattoConfiguration.allowed_users.env_name,
-    )
-
     ctx.register_platform(
         name=ChattoConstants.PLATFORM_NAME,  # this will be the config.yaml key.
         label=ChattoConstants.PLATFORM_LABEL,