Sfoglia il codice sorgente

Make the plugin tools work and keep them inside the platform's rules

chatto_send_dm, chatto_send_room and chatto_list_rooms shipped unusable and
unsafe. Every handler returned a dict and was registered without is_async, so
Hermes' dispatcher answered "unsupported result type: coroutine" on the first
call — the plugin advertised three tools that could not run. Both send tools
took base_url and token as arguments, and since an omitted token falls back to
the configured secret, any message the bot read could talk the model into
sending the bot token as a bearer credential to a host it named; the branch's
own skill file states the rule these schemas break. They posted into channels
the behavioural contract calls read-only, opened DM rooms the server refuses
bots to open, reported a half-delivered send as a plain failure that invites a
duplicating retry, picked the first of several equally named rooms, and sent
thread roots without checking they belonged to the room. Not one of them had a
behavioural test; the whole test delta was four lines of registration mock.

Credentials now come from the resolved configuration only, early exits raise
_ToolRefusal so every path agrees on the result shape and the handlers are
annotated -> str (a regression to dict becomes a type error rather than a
runtime break), chunk failures report what went out, a room name must be
unambiguous, a thread root must live in the destination room, and the room
gate mirrors ChattoAdapter._room_policy so the read-only promise holds for
tools as well. ConfigValue.explicit no longer counts a blank value as user
intent, which had been defeating the cron-session fallback it exists to enable,
and the fallback reads config.yaml's own extra block instead of an empty one.

start_dm is removed outright rather than repaired: Chatto lets only a person
start a DM, Hermes never calls such a method (BasePlatformAdapter does not
declare one), and the vendored client could not deliver it anyway — the
AttributeError was swallowed into a silent None under a "DM initiation"
capability banner. Its slot in the docs now names the real path.
Paul Klumpp 2 settimane fa
parent
commit
084a32be6e
8 ha cambiato i file con 1069 aggiunte e 437 eliminazioni
  1. 4 1
      AGENTS.md
  2. 23 2
      README.md
  3. 490 401
      adapter.py
  4. 6 0
      after-install.md
  5. 39 7
      platform_config.py
  6. 12 9
      skills/chatto-platform-plugin/SKILL.md
  7. 423 17
      test_adapter.py
  8. 72 0
      test_platform_config.py

+ 4 - 1
AGENTS.md

@@ -148,7 +148,10 @@ changes must keep them, or consciously renegotiate the docs.**
   share `_mentions_me`, so "addressed" means the same thing everywhere:
   case-insensitive handle matching outside code regions, per the Chatto web
   frontend's extraction. DMs answer regardless so `/join` stays reachable;
-  `hermes_validate_config` rejects listing a room on both lists.
+  `hermes_validate_config` rejects listing a room on both lists. The plugin
+  tools obey the same lists: `chatto_send_room` refuses a channel on neither
+  (a DM room is never gated), because a tool that posted there would break the
+  read-only promise the same paragraph makes.
   `CHATTO_HOME_CHANNEL` is orthogonal: it is the default outbound target for
   context-less cron/notification delivery, not an answer destination.
 - **Threads stay threads.** An inbound thread reply keeps its thread context;

+ 23 - 2
README.md

@@ -41,7 +41,8 @@ Capabilities natively implemented by the Chatto plugin adapter:
 | processing notifications | yes (👀/✅/❌/🚫) |
 | file attachments | yes (chunked upload) |
 | image / video / audio / documents | yes (native attachments) |
-| DM initiation | yes |
+| DM participation | only once the person has DMed the bot — a bot cannot start a DM on Chatto, see [DM Initiation](#dm-initiation) |
+| plugin tools | yes (`chatto_list_rooms`, `chatto_send_room`, `chatto_send_dm` — see [Plugin tools](#plugin-tools)) |
 | room creation | yes |
 | room join/leave over DM | yes (`/join`, `/leave`) |
 | member directory | yes (cached) |
@@ -269,7 +270,7 @@ After discovering rooms, the adapter marks joined rooms as read and dismisses pe
 
 ### DM Initiation
 
-The adapter can proactively start direct messages with other Chatto users via the `StartDM` RPC endpoint. This is used for cron job delivery and notifications.
+On Chatto only a person can start a direct message — a bot cannot, not even with `message.post`. So a DM room exists only once the other user has written to the bot at least once; until then the bot has no room to post into, and no setting changes that. The adapter therefore has no way to open a DM at all: it joins *rooms* on request (`/join <room-id or #name>` in a DM, see [Managing Rooms over DM](#managing-rooms-over-dm)), and the `chatto_send_dm` plugin tool reports the precondition rather than pretending to deliver.
 
 ### Room Creation
 
@@ -355,6 +356,26 @@ A channel `/join` reply reports the room's mention-list status: an unlisted room
 
 Two refusals by design: direct messages cannot be left, and leaving the configured home channel is rejected because cron/notification delivery posts there. If `CHATTO_HOME_CHANNEL` names a room the bot has not joined, the adapter logs a warning at each reconnect — invite the account natively in Chatto or DM it `/join`.
 
+## Plugin tools
+
+Beyond answering inbound messages, the agent can act on Chatto on its own initiative. The plugin registers three tools, available to the agent in the gateway and, with `hermes tools`, in the CLI:
+
+| Tool | What it does |
+|------|--------------|
+| `chatto_list_rooms` | Lists the rooms visible to the bot with their IDs, kinds and membership. Optional `name` filters case-insensitively by substring. Useful for turning a channel name into the `platform:chat_id` address a cron `deliver` target needs. |
+| `chatto_send_room` | Posts a message into a channel or DM room, by `room_id` or `room_name`. Can post inside an existing thread (`thread_root_event_id`) or open a new one (`create_thread`). |
+| `chatto_send_dm` | Posts a message into the DM room you already have with a person, addressed by `user_id` or `user_login`. |
+
+Three boundaries are deliberate, and each tool reports them as an error instead of half-doing the job:
+
+- **A channel must be opted in.** `chatto_send_room` only posts into a room listed in `CHATTO_REQUIRE_MENTION_ROOMS` or `CHATTO_OPTIONAL_MENTION_ROOMS` (DM rooms are never gated). A room on neither list stays read-only — a human message there would never have produced output either. The refusal names both settings so you can paste the room ID into `~/.hermes/.env`.
+- **The bot cannot open a DM.** Only a person can start a direct message on Chatto, so `chatto_send_dm` needs a recipient who has written to the bot at least once. To reach someone else, mention them in a channel message.
+- **A name is not an identity.** If several rooms share the name you passed, `chatto_send_room` returns their IDs and asks for `room_id` instead of picking one. Same rule as `/join #name`.
+
+Credentials never come from the model: no tool takes a server URL or a token as an argument, so the configured bot token cannot be pointed at another host. Inside a channel, mention people in the message text (`@login, …`) — there is no separate mention parameter.
+
+Long messages are split at the same limit as every other send (see [Message Splitting](#message-splitting)). If a chunk fails after earlier ones were delivered, the tool reports what it already sent plus a `partial: true` marker, so a retry can repeat only the missing part.
+
 ## Troubleshooting
 
 **First step for every issue below:** look at what the gateway logged around the time things went wrong:

File diff suppressed because it is too large
+ 490 - 401
adapter.py


+ 6 - 0
after-install.md

@@ -72,3 +72,9 @@ can live in either file. Environment variables take precedence over
    5 minutes and Hermes restarts his answer with the corrected text. If he had
    ignored the message because an `@mention` was missing, editing the mention
    in makes him answer after all.
+
+   The bot can also talk first: ask it to post a summary into a channel it
+   answers in, or to DM someone who has already written to it. Those are the
+   `chatto_send_room` / `chatto_send_dm` tools, which obey the same room rules
+   as inbound messages — a channel listed in neither mention room list stays
+   read-only. `hermes tools` lists all three.

+ 39 - 7
platform_config.py

@@ -18,6 +18,7 @@ except ImportError:  # pragma: no cover - depends on how the module is loaded
 setup_vendor_path()
 
 import logging
+from collections.abc import Mapping
 from typing import Any, ClassVar, Generic, TypeVar
 
 import utils
@@ -243,7 +244,7 @@ class ConfigValue(Generic[T]):
     the actual setting instead of the always-truthy wrapper object.
     """
 
-    __slots__ = ("env_name", "field_name", "value", "explicit")
+    __slots__ = ("env_name", "explicit", "field_name", "value")
 
     def __init__(
         self, value: T, field_name: str, env_name: str, explicit: bool = False
@@ -330,23 +331,29 @@ class ConfigField(Generic[T]):
     def resolve(self, extra: dict[str, Any]) -> ConfigValue:
         """Read this field from the environment, then from ``extra``, then the default."""
         raw = extra.get(self.config_key)
-        raw_present = raw is not None
+        # "The user set it somewhere" — a blank value still counts as set here,
+        # because for a bool that is the only way to tell an explicit "false"
+        # from the declared default.
+        raw_present = os.getenv(self.env_name) is not None or raw is not None
 
         if self.kind == "list":
             value: Any = _get_env_or_extra_list(self.env_name, raw)
-            explicit = os.getenv(self.env_name) is not None or raw_present
         elif self.kind == "bool":
             value = _get_env_or_extra_truthy(self.env_name, raw, bool(self.default))
-            explicit = os.getenv(self.env_name) is not None or raw_present
         elif self.kind == "int":
             value = _get_env_or_extra_int(self.env_name, raw, int(self.default))
-            explicit = os.getenv(self.env_name) is not None or raw_present
         elif self.kind == "str_opt":
             value = _get_env_or_extra_str_opt(self.env_name, raw, self.default)
-            explicit = os.getenv(self.env_name) is not None or raw_present
         else:
             value = _get_env_or_extra_str(self.env_name, raw, self.default)
-            explicit = os.getenv(self.env_name) is not None or raw_present
+
+        # A blank value is a placeholder, not intent: `CHATTO_TOKEN=` left
+        # behind by the installer must not count as "the user set the token",
+        # or the tool-layer fallback — which yields to explicit values — would
+        # never reach the real secret in the profile's .env. A bool is never
+        # blank, so `False` there stays explicit.
+        blank = value is None or value == "" or value == []
+        explicit = raw_present and not blank
 
         return ConfigValue(value, self.field_name, self.env_name, explicit=explicit)
 
@@ -405,6 +412,31 @@ class ChattoConfiguration:
         """All declared config fields, in declaration order."""
         return [v for v in vars(cls).values() if isinstance(v, ConfigField)]
 
+    def overlay_profile_secrets(self, secrets: Mapping[str, str]) -> None:
+        """Fill every field that is not an explicit user value from ``secrets``.
+
+        Keyed by env var name. The plugin tools call this with the profile's own
+        .env, which a cron or CLI process no longer sees through the
+        environment: there, a secret would otherwise resolve to its default and
+        the tool would report "token not configured" for a configured bot.
+
+        An EXPLICIT value (env var or config.yaml extra) is the user's own
+        intent and wins; a declared default or a blank placeholder yields here.
+        """
+        for config_field in self.fields():
+            current = self._values.get(config_field.field_name)
+            if current is not None and current.explicit:
+                continue
+            secret_value = secrets.get(config_field.env_name)
+            if not secret_value:
+                continue
+            self._values[config_field.field_name] = ConfigValue(
+                secret_value,
+                config_field.field_name,
+                config_field.env_name,
+                explicit=True,
+            )
+
     def __str__(self) -> str:
         redacted = {"token"}
         parts = [

+ 12 - 9
skills/chatto-platform-plugin/SKILL.md

@@ -19,17 +19,18 @@ Using or developing/debugging the Chatto gateway plugin: its plugin tools (`chat
 
 ### chatto_list_rooms(name?) — resolve room names to IDs
 
-Lists every room the bot is a member of: `room_id`, `name`, `kind` (channel/DM), `threading_mode`, `archived`. Optional `name` filters case-insensitively by substring (unnamed DM rooms are skipped when a filter is set; without a filter they appear with an empty name and label `(DM room <id>)`).
+Lists every room visible to the bot: `room_id`, `name`, `kind` (channel/DM), `threading_mode`, `archived`, `is_member`, `label`. Optional `name` filters case-insensitively by substring (unnamed DM rooms are skipped when a filter is set; without a filter they appear with an empty name and label `(DM room <id>)`).
 
 **Primary use case: cron `deliver` targets.** The Hermes cron `deliver` field takes only `platform:chat_id` strings and does NO name resolution. When a scheduled job's report should go to a channel, call this tool first, take the `room_id`, and use it as the `chat_id` part. Never trust chat names in session/origin metadata as room IDs — resolve live.
 
 ### chatto_send_room(room_id|room_name, message, create_thread?, thread_root_event_id?) — post into a channel
 
-Resolves the room case-insensitively by name OR verifies the ID, then posts. Works for any room the bot is a member of.
+Resolves the room by ID or case-insensitively by name, then posts. A name shared by several rooms is refused with the candidate IDs rather than guessed (`_resolve_room`).
 
-- `create_thread=true` posts the message as the ROOT of a NEW thread (first chunk = root, further chunks inside), then best-effort `follow_thread`.
-- `thread_root_event_id` posts INSIDE an existing thread. Mutually exclusive with `create_thread`.
+- `create_thread=true` posts the message as the ROOT of a NEW thread (first chunk = root, further chunks inside), then best-effort `follow_thread`. Channel rooms only — a DM has no threads.
+- `thread_root_event_id` posts INSIDE an existing thread; the root must be a message of that same room (`client.get_message`), otherwise the call refuses.
 - Response carries `thread_root_event_id` + `created_thread`.
+- **Only rooms the bot answers in**: a channel has to be on `CHATTO_REQUIRE_MENTION_ROOMS` or `CHATTO_OPTIONAL_MENTION_ROOMS`, same as inbound messages (`_room_is_writable`); DM rooms are never gated. The refusal names both env vars because that reply is where an operator copies the room ID from.
 
 **Mentions instead of DMs:** to notify a user inside a channel, mention them in the message TEXT (`@login, hier ist …`) — Chatto renders room-text mentions. A bot cannot deliver a personal notification any other way (see the DM limitation below).
 
@@ -37,7 +38,7 @@ Resolves the room case-insensitively by name OR verifies the ID, then posts. Wor
 
 Looks up the user by login or ID and posts into the DM room.
 
-**Precondition: the recipient must have DMed the bot once.** Bots can NEVER start a DM — by design (Chatto dev-docs, guides/operations/permissions: "Only people can start a DM... A bot cannot start a DM, even with message.post."). No permission or role change lifts this. On `permission_denied`: the recipient must message the bot first, then the existing-room path works.
+**Precondition: the recipient must have DMed the bot once.** Bots can NEVER start a DM — by design (Chatto dev-docs, guides/operations/permissions: "Only people can start a DM... A bot cannot start a DM, even with message.post."). No permission or role change lifts this. The handler therefore never calls `start_dm`: with no existing DM room it refuses and says so (`_find_dm_room`). A login is resolved among the members of rooms the bot shares with that person (`_find_user_by_login`), because reading the server-wide user directory needs admin rights.
 
 ## Cron delivery patterns
 
@@ -48,9 +49,11 @@ Looks up the user by login or ID and posts into the DM room.
 ## Plugin tool contract (ctx.register_tool) — for extending the plugin
 
 - **Async handlers need `is_async=True`** — the default is sync; an `async def` handler registered without it returns an unawaited coroutine and the dispatcher fails with `tool_result_contract: unsupported result type: coroutine` (`tools/registry.py: dispatch()`).
-- **Handlers must return JSON strings** (`json.dumps({...})`), never plain dicts — `_normalize_handler_result` accepts only str or the multimodal envelope.
-- **Config access: never read `os.environ` in handlers.** `.env` credentials like `CHATTO_TOKEN` are profile-scoped and NOT visible in cron fires' os.environ, and `hermes_validate_config` never runs in a fresh cron session. Resolve config through `_chatto_config_for_tools()` in adapter.py: (1) module-level `_resolved_chatto_config` set by `hermes_validate_config` in a running gateway; (2) empty `PlatformConfig(enabled=True, extra={})` wrapped in `ChattoConfiguration`, re-filled from `build_profile_secret_scope(get_hermes_home())` (`agent.secret_scope`) — re-reads the profile's own `.env`, works in gateway, cron and CLI alike; (3) clean error. Do NOT use `load_gateway_config().platforms[name]` as fallback (returns None in fresh processes) and do NOT leak a `token` parameter into the tool schema.
-- **Reuse `_chatto_tool_client(args, cfg)`** for client construction in new handlers instead of re-inlining token/base_url resolution.
+- **Handlers must return JSON strings** (`json.dumps({...})`), never plain dicts — `_normalize_handler_result` accepts only str or the multimodal envelope. Annotate them `-> str` too: the annotation is what makes a regression to `dict` a type error instead of a runtime break.
+- **Early exits raise `_ToolRefusal`, they do not return.** The `async with _tool_client(cfg)` context manager and the `_refuse(...)` helper keep one result shape per failure; the `_handle_*` wrapper turns a refusal back into its JSON and catches `ChattoError`/`ChattoAuthError` around it. Never `except Exception: pass` — the AGENTS.md rule about naming each recovery applies here too.
+- **Config access: never read `os.environ` in handlers.** `.env` credentials like `CHATTO_TOKEN` are profile-scoped and NOT visible in cron fires' os.environ, and `hermes_validate_config` never runs in a fresh cron session. Resolve config through `_chatto_config_for_tools()` in adapter.py: (1) module-level `_resolved_chatto_config`, installed by `hermes_validate_config` only after it accepted the config; (2) a fresh `load_gateway_config()` read for the `chatto-platform` platform entry, so `config.yaml`'s own `extra` block is honoured, re-filled from `build_profile_secret_scope(get_hermes_home())` (`agent.secret_scope`) — re-reads the profile's own `.env`, works in gateway, cron and CLI alike; (3) clean error.
+- **Never put a credential or a server URL in a tool schema.** The configured bot token must not be reachable from an argument: a `base_url` parameter would send it as `Authorization: Bearer …` to whatever host the caller names, and any message the bot reads is something the model could be talked into repeating. Take the client from `async with _tool_client(cfg) as client:` — it is the only sanctioned way to build one.
+- **A half-delivered send says so.** `_post_chunks` reports `partial: true` plus the `message_ids` that went out, so a retry repeats only what is missing. Keep that contract when adding a send path.
 - **Verify in BOTH contexts:** a gateway-session `tool_call` is not enough — also fire a `cronjob_manage action='run'` test when the tool will run from cron, because cron sessions exercise the config-fallback tier gateway sessions never hit. Debug order: `tool_result_contract` error → contract bug (async/return type); `AttributeError` on the client → wrong stub/service name; `permission_denied` → server-side limit.
 
 ## Development workflow
@@ -58,4 +61,4 @@ Looks up the user by login or ID and posts into the DM room.
 1. **Read the repo's `AGENTS.md` first** — it is the binding rulebook for all work on this repo: editing, linting/type-checking gates, shell-script checks, typed access, overriding, capabilities, README discipline. This skill does not repeat those rules.
 2. **Gateway restart is REQUIRED after plugin code changes** — the running Hermes session keeps the old module in memory. Ask the user to restart, then verify via `tool_call`.
 3. **Before requesting a restart, verify config-dependent fixes in an isolated subprocess** simulating the cron scope: pop the relevant env vars (`CHATTO_TOKEN`, `CHATTO_BASE_URL`), `sys.path.insert` the Hermes kernel and plugin dir, set `HERMES_HOME`, call the handler directly. A green isolated test proves the cron path before burning a restart cycle.
-4. **Bundled skills:** register additional skills in `register(ctx)` via `ctx.register_skill(name, path_to_SKILL.md)` — see the `skills/` directory of this plugin.
+4. **Bundled skills:** add a `skills/<name>/SKILL.md` and register it via `_register_bundled_skills(ctx)`, which passes the SKILL.md frontmatter `description` through — `register_skill` stores it verbatim and does not read the frontmatter itself, so an empty description leaves the skill listed with nothing explaining when to load it.

+ 423 - 17
test_adapter.py

@@ -14,6 +14,8 @@ All network calls are mocked — no real HTTP or WebSocket connections.
 """
 
 import asyncio
+import inspect
+import json
 import os
 import sys
 from datetime import UTC, datetime, timedelta
@@ -68,10 +70,17 @@ from gateway.platforms.base import (
 )
 
 from adapter import (
+    CHATTO_LIST_ROOMS_SCHEMA,
+    CHATTO_SEND_DM_SCHEMA,
+    CHATTO_SEND_ROOM_SCHEMA,
     ChattoAdapter,
     HermesChatType,
     RoomPolicy,
     _capabilities,
+    _chatto_config_for_tools,
+    _handle_chatto_list_rooms,
+    _handle_chatto_send_dm,
+    _handle_chatto_send_room,
     chat_type_for_room_kind,
     hermes_env_enablement_fn,
     hermes_standalone_sender_fn,
@@ -83,7 +92,7 @@ from adapter import (
 from adapter import (
     hermes_validate_config as validate_config,
 )
-from platform_config import ChattoConstants
+from platform_config import ChattoConfiguration, ChattoConstants
 
 _EMOJI_TO_SHORTCODE = ChattoConstants.EMOJI_TO_SHORTCODE
 _MAX_MESSAGE_LENGTH = ChattoConstants.MAX_MESSAGE_LENGTH
@@ -99,11 +108,17 @@ class _MockPluginContext:
     def __init__(self):
         self.registered_names = []
         self.registered_kwargs = None
+        self.registered_tools = []
+        self.registered_skills = []
 
     def register_tool(self, **kwargs):
         """Record plugin-tool registrations (chatto_send_dm / chatto_send_room)."""
+        self.registered_tools.append(kwargs)
         self.registered_names.append(kwargs.get("name", "<tool>"))
 
+    def register_skill(self, name, path, **kwargs):
+        self.registered_skills.append((name, path, kwargs))
+
     def register_platform(self, **kwargs):
         from gateway.platform_registry import PlatformEntry, platform_registry
 
@@ -2767,7 +2782,7 @@ class TestInboundAttachments:
 
 
 class TestRoomOperations:
-    """Test room creation and DM initiation."""
+    """Test room creation."""
 
     @pytest_asyncio.fixture
     def adapter(self):
@@ -2775,11 +2790,10 @@ class TestRoomOperations:
         cfg = _make_config()
         adapter = ChattoAdapter(cfg)
         adapter._chatto_client = MagicMock()
-        # AsyncMock, not MagicMock: the adapter awaits these, and awaiting a
-        # plain MagicMock raises TypeError, which create_room()/start_dm()
-        # swallow into a None return.
+        # AsyncMock, not MagicMock: the adapter awaits this, and awaiting a
+        # plain MagicMock raises TypeError, which create_room() swallows into a
+        # None return.
         adapter._chatto_client.create_room = AsyncMock()
-        adapter._chatto_client.start_dm = AsyncMock()
         adapter._room_names = {}
         adapter._room_kinds = {}
         return adapter
@@ -2795,17 +2809,6 @@ class TestRoomOperations:
         adapter._chatto_client.create_room.assert_called_once()
         assert adapter._room_names["room-123"] == "Test Room"
 
-    async def test_start_dm(self, adapter):
-        adapter._chatto_client.start_dm.return_value = _make_room(
-            "dm-123",
-            "DM with user",
-            RoomKind.DM,
-        )
-        result = await adapter.start_dm("user-123")
-        assert result == "dm-123"
-        adapter._chatto_client.start_dm.assert_called_once()
-        assert adapter._room_kinds["dm-123"] == RoomKind.DM
-
 
 # -- DM room management (/join, /leave) --
 
@@ -3355,3 +3358,406 @@ class TestStandaloneSender:
         assert result.success is True
         for call in client.post_message.await_args_list:
             assert call.kwargs["thread_root_event_id"] == "root-9"
+
+
+# -- Plugin tools (chatto_list_rooms / chatto_send_dm / chatto_send_room) --
+
+
+def _make_room_state(room, is_member=True, member_user_ids=None):
+    """RoomWithViewerState as list_rooms()/get_room() return it, with membership."""
+    return RoomWithViewerState(
+        room=room,
+        viewer_state=RoomViewerState(is_member=is_member),
+        member_user_ids=list(member_user_ids or []),
+    )
+
+
+class TestPluginToolSchemas:
+    """The schemas are the model's only view of these tools — they must not
+    offer credentials, and registration must satisfy the handler contract."""
+
+    @pytest.mark.parametrize(
+        "schema",
+        [CHATTO_LIST_ROOMS_SCHEMA, CHATTO_SEND_DM_SCHEMA, CHATTO_SEND_ROOM_SCHEMA],
+        ids=["list_rooms", "send_dm", "send_room"],
+    )
+    def test_schema_advertises_no_credentials(self, schema):
+        """A base_url argument would send the configured bot token to a host
+        the model names — one injected message away from full credential loss."""
+        properties = schema["parameters"]["properties"]
+        assert "token" not in properties
+        assert "base_url" not in properties
+
+    def test_every_tool_is_registered_as_an_async_handler(self):
+        """Hermes only awaits a handler whose entry is flagged async, and only
+        accepts a string result — an unflagged coroutine is dead on arrival."""
+        ctx = _MockPluginContext()
+        register(ctx)
+        assert [tool["name"] for tool in ctx.registered_tools] == [
+            "chatto_send_dm",
+            "chatto_send_room",
+            "chatto_list_rooms",
+        ]
+        for tool in ctx.registered_tools:
+            assert tool["is_async"] is True, tool["name"]
+            assert inspect.iscoroutinefunction(tool["handler"]), tool["name"]
+
+    def test_bundled_skill_is_registered_with_a_description(self):
+        """A skill registered without a description is invisible in the
+        available-skills list, so nothing ever loads it."""
+        ctx = _MockPluginContext()
+        register(ctx)
+        assert ctx.registered_skills, "no bundled skill registered"
+        for name, path, kwargs in ctx.registered_skills:
+            assert kwargs.get("description"), f"{name} registered without description"
+            assert os.path.exists(path), path
+
+
+class TestPluginToolHandlers:
+    """The handlers run outside the adapter, so they resolve config and build
+    their own client — every path through them has to hold the platform's
+    promises on its own."""
+
+    def _stub_client(self):
+        """A ChattoClient stand-in whose every call succeeds by default."""
+        client = MagicMock()
+        client.close = AsyncMock()
+        client.me = AsyncMock(return_value=_make_user("bot-user-id", "hermes_bot"))
+        client.list_rooms = AsyncMock(return_value=[])
+        client.get_room = AsyncMock(return_value=None)
+        client.get_message = AsyncMock(return_value=None)
+        client.list_room_members = AsyncMock(return_value=([], Page()))
+        client.batch_get_room_members = AsyncMock(return_value=[])
+        client.follow_thread = AsyncMock()
+        client.start_dm = AsyncMock()
+        client.post_message = AsyncMock(
+            side_effect=lambda *a, **kw: _make_message(
+                message_id=f"msg-{client.post_message.await_count}"
+            )
+        )
+        return client
+
+    async def _run(self, handler, args, *, client=None, **config_extra):
+        """Call a tool handler with a stubbed client; returns (parsed, client)."""
+        stub = self._stub_client() if client is None else client
+        cfg = ChattoConfiguration(
+            pconfig=_make_config(token="test-token", **config_extra)
+        )
+        with (
+            patch("adapter.ChattoClient", return_value=stub) as client_cls,
+            patch("adapter._chatto_config_for_tools", return_value=cfg),
+        ):
+            raw = await handler(args)
+            return json.loads(raw), stub, client_cls
+
+    # -- credentials never come from the model --
+
+    async def test_injected_credentials_are_ignored(self):
+        """base_url/token in args must not reach the client: the configured
+        token is the one the plugin holds, and the configured server its own."""
+        room = _make_room("room-1", "Ops", RoomKind.CHANNEL)
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        _, _, client_cls = await self._run(
+            _handle_chatto_send_room,
+            {
+                "room_id": "room-1",
+                "message": "hi",
+                "base_url": "https://attacker.example",
+                "token": "cht_ATTACKER",
+            },
+            client=client,
+            optional_mention_rooms="room-1",
+        )
+        assert client_cls.call_args.kwargs["base_url"] == "https://chat.example.com"
+        assert client_cls.call_args.kwargs["token"] == "test-token"
+
+    # -- DM rooms --
+
+    def _dm_room_with(self, user_id):
+        """One DM room between the bot and ``user_id``, as list_rooms() reports it."""
+        return _make_room_state(
+            _make_room(f"dm-{user_id}", "", RoomKind.DM),
+            member_user_ids=["bot-user-id", user_id],
+        )
+
+    async def test_send_dm_uses_the_existing_dm_room(self):
+        client = self._stub_client()
+        client.list_rooms = AsyncMock(return_value=[self._dm_room_with("user-1")])
+        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_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."""
+        client = self._stub_client()
+        client.list_rooms = AsyncMock(return_value=[self._dm_room_with("user-1")])
+        client.list_room_members = AsyncMock(return_value=(["user-1"], Page()))
+        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"
+        assert result["dm_room_id"] == "dm-user-1"
+
+    async def test_send_dm_does_not_search_channels_for_a_login(self):
+        """Only a DM room can carry the reply, so scanning every channel the
+        bot sits in buys nothing but RPCs — and can resolve the wrong person."""
+        target = _make_user("user-1", "Alice")
+        client = self._stub_client()
+        channel = _make_room_state(
+            _make_room("room-1", "Ops", RoomKind.CHANNEL), member_user_ids=["user-1"]
+        )
+        client.list_rooms = AsyncMock(
+            return_value=[channel, self._dm_room_with("user-1")]
+        )
+        client.list_room_members = AsyncMock(return_value=(["user-1"], Page()))
+        client.batch_get_room_members = AsyncMock(
+            return_value=[DirectoryMember(user=target)]
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_send_dm,
+            {"user_login": "alice", "message": "hi"},
+            client=client,
+        )
+        assert result["success"] is True
+        assert result["dm_room_id"] == "dm-user-1"
+        searched = [call.args[0] for call in client.list_room_members.await_args_list]
+        assert searched == ["dm-user-1"]
+
+    async def test_send_dm_refuses_to_open_a_dm_room(self):
+        """Only people may start a DM (Chatto permissions guide), so a recipient
+        who has never written to the bot is unreachable — say so, don't guess."""
+        client = self._stub_client()
+        client.list_rooms = AsyncMock(
+            return_value=[
+                _make_room_state(_make_room("room-1", "Ops", RoomKind.CHANNEL))
+            ]
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_send_dm,
+            {"user_id": "user-1", "message": "hi"},
+            client=client,
+        )
+        assert result["success"] is False
+        assert "user-1" in result["error"]
+        client.start_dm.assert_not_awaited()
+        client.post_message.assert_not_awaited()
+
+    # -- room policy --
+
+    async def test_send_room_refuses_a_read_only_room(self):
+        """A channel on neither mention list is read-only by contract — the
+        send path must not become a way around that."""
+        room = _make_room("room-1", "Ops", RoomKind.CHANNEL)
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        result, _, _ = await self._run(
+            _handle_chatto_send_room,
+            {"room_id": "room-1", "message": "hi"},
+            client=client,
+        )
+        assert result["success"] is False
+        assert "CHATTO_REQUIRE_MENTION_ROOMS" in result["error"]
+        assert "CHATTO_OPTIONAL_MENTION_ROOMS" in result["error"]
+        client.post_message.assert_not_awaited()
+
+    async def test_send_room_posts_into_a_listed_room(self):
+        room = _make_room("room-1", "Ops", RoomKind.CHANNEL)
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        result, _, _ = await self._run(
+            _handle_chatto_send_room,
+            {"room_id": "room-1", "message": "hi"},
+            client=client,
+            require_mention_rooms="room-1",
+        )
+        assert result["success"] is True
+
+    async def test_send_room_posts_into_a_dm_room_that_is_on_no_list(self):
+        """DMs answer regardless of the mention lists, so they stay writable."""
+        room = _make_room("dm-1", "", RoomKind.DM)
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        result, _, _ = await self._run(
+            _handle_chatto_send_room,
+            {"room_id": "dm-1", "message": "hi"},
+            client=client,
+        )
+        assert result["success"] is True
+
+    # -- room lookup --
+
+    async def test_send_room_rejects_an_ambiguous_room_name(self):
+        """Two channels can share a name; delivering to whichever the directory
+        happened to list first is not a choice anyone made."""
+        client = self._stub_client()
+        client.list_rooms = AsyncMock(
+            return_value=[
+                _make_room_state(_make_room("room-1", "Liv", RoomKind.CHANNEL)),
+                _make_room_state(_make_room("room-2", "liv", RoomKind.CHANNEL)),
+            ]
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_send_room,
+            {"room_name": "liv", "message": "hi"},
+            client=client,
+        )
+        assert result["success"] is False
+        assert "room-1" in result["error"] and "room-2" in result["error"]
+        client.post_message.assert_not_awaited()
+
+    # -- threading --
+
+    async def test_send_room_refuses_to_thread_in_a_dm(self):
+        """A DM conversation has no threads — the adapter never opens one."""
+        room = _make_room("dm-1", "", RoomKind.DM)
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        result, _, _ = await self._run(
+            _handle_chatto_send_room,
+            {"room_id": "dm-1", "message": "hi", "create_thread": True},
+            client=client,
+        )
+        assert result["success"] is False
+        client.post_message.assert_not_awaited()
+
+    async def test_send_room_refuses_a_thread_root_from_another_room(self):
+        """A stale or foreign root must not silently attach the message to a
+        thread somewhere else."""
+        room = _make_room("room-1", "Ops", RoomKind.CHANNEL)
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        client.get_message = AsyncMock(return_value=None)
+        result, _, _ = await self._run(
+            _handle_chatto_send_room,
+            {"room_id": "room-1", "message": "hi", "thread_root_event_id": "msg-9"},
+            client=client,
+            optional_mention_rooms="room-1",
+        )
+        assert result["success"] is False
+        assert "msg-9" in result["error"]
+        client.post_message.assert_not_awaited()
+
+    async def test_send_room_accepts_a_thread_root_in_the_same_room(self):
+        room = _make_room("room-1", "Ops", RoomKind.CHANNEL)
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        client.get_message = AsyncMock(
+            return_value=_make_message(message_id="msg-9", room_id="room-1")
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_send_room,
+            {"room_id": "room-1", "message": "hi", "thread_root_event_id": "msg-9"},
+            client=client,
+            optional_mention_rooms="room-1",
+        )
+        assert result["success"] is True
+        assert client.post_message.await_args.kwargs["thread_root_event_id"] == "msg-9"
+
+    # -- chunked delivery --
+
+    async def test_send_room_reports_a_partial_delivery(self):
+        """Chunks already sent are out in the world; reporting total failure
+        invites a retry that sends them twice."""
+        room = _make_room("room-1", "Ops", RoomKind.CHANNEL)
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        client.post_message = AsyncMock(
+            side_effect=[_make_message(message_id="msg-1"), ChattoError("rate limited")]
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_send_room,
+            {"room_id": "room-1", "message": "x" * 12000},
+            client=client,
+            optional_mention_rooms="room-1",
+        )
+        assert result["success"] is False
+        assert result["partial"] is True
+        assert result["message_ids"] == ["msg-1"]
+        assert result["chunks_sent"] == 1
+        assert "rate limited" in result["error"]
+
+    async def test_send_dm_reports_a_partial_delivery(self):
+        client = self._stub_client()
+        client.list_rooms = AsyncMock(return_value=[self._dm_room_with("user-1")])
+        client.post_message = AsyncMock(
+            side_effect=[_make_message(message_id="msg-1"), ChattoError("rate limited")]
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_send_dm,
+            {"user_id": "user-1", "message": "x" * 12000},
+            client=client,
+        )
+        assert result["success"] is False
+        assert result["partial"] is True
+        assert result["message_ids"] == ["msg-1"]
+
+    # -- config resolution --
+
+    # -- list_rooms --
+
+    def _directory(self):
+        """A directory with a named channel, an archived one and a DM room."""
+        return [
+            _make_room_state(_make_room("room-1", "Liv", RoomKind.CHANNEL)),
+            _make_room_state(_make_room("room-2", "Liv-Dev", RoomKind.CHANNEL)),
+            _make_room_state(
+                _make_room("dm-1", "", RoomKind.DM), member_user_ids=["u1", "u2"]
+            ),
+        ]
+
+    async def test_list_rooms_reports_ids_kinds_and_membership(self):
+        client = self._stub_client()
+        client.list_rooms = AsyncMock(return_value=self._directory())
+        result, _, _ = await self._run(_handle_chatto_list_rooms, {}, client=client)
+        assert result["success"] is True
+        assert result["count"] == 3
+        by_id = {room["room_id"]: room for room in result["rooms"]}
+        assert by_id["room-1"]["name"] == "Liv"
+        assert by_id["room-1"]["is_member"] is True
+        assert by_id["room-1"]["kind"] == "ROOM_KIND_CHANNEL"
+        # A DM room has no name, so it is labelled by its ID instead of blank.
+        assert by_id["dm-1"]["label"] == "(DM room dm-1)"
+        assert by_id["dm-1"]["name"] == ""
+
+    async def test_list_rooms_name_filter_skips_dm_rooms(self):
+        client = self._stub_client()
+        client.list_rooms = AsyncMock(return_value=self._directory())
+        result, _, _ = await self._run(
+            _handle_chatto_list_rooms, {"name": "liv"}, client=client
+        )
+        assert [room["room_id"] for room in result["rooms"]] == ["room-1", "room-2"]
+
+    async def test_tool_config_falls_back_to_config_yaml(self):
+        """A cron session never runs hermes_validate_config, so the fallback
+        has to read config.yaml's own extra block — not an empty one."""
+        import gateway.config as gw_config
+
+        yaml_cfg = gw_config.PlatformConfig(
+            enabled=True,
+            extra={"token": "yaml-token", "base_url": "https://yaml.example"},
+        )
+        gateway_cfg = gw_config.GatewayConfig(
+            platforms={Platform(ChattoConstants.PLATFORM_NAME): yaml_cfg}
+        )
+        with (
+            patch("adapter._resolved_chatto_config", None),
+            patch.object(gw_config, "load_gateway_config", return_value=gateway_cfg),
+            # None in sys.modules makes the import fail, as in a bare cron process.
+            patch.dict(sys.modules, {"agent.secret_scope": None}),
+        ):
+            resolved = _chatto_config_for_tools()
+        assert resolved is not None
+        assert resolved.token.value == "yaml-token"
+        assert resolved.base_url.value == "https://yaml.example"

+ 72 - 0
test_platform_config.py

@@ -157,6 +157,78 @@ class TestMentionRoomFields:
         assert cfg.require_mention_rooms.value == ["room-1", "room-2"]
 
 
+class TestExplicitOrigin:
+    """`explicit` marks a value the user actually set, so the tool fallback can
+    tell their intent from a default. A blank value is a placeholder — treating
+    it as intent would block the profile-.env fallback it exists to enable."""
+
+    def test_env_value_is_explicit(self, monkeypatch):
+        monkeypatch.setenv("CHATTO_TOKEN", "cht_x")
+        cfg = ChattoConfiguration(pconfig=PlatformConfigStub(extra={}))
+        assert cfg.token.explicit is True
+
+    def test_blank_env_value_is_not_explicit(self, monkeypatch):
+        monkeypatch.setenv("CHATTO_TOKEN", "")
+        cfg = ChattoConfiguration(pconfig=PlatformConfigStub(extra={}))
+        assert cfg.token.explicit is False
+
+    def test_blank_extra_value_is_not_explicit(self):
+        cfg = ChattoConfiguration(pconfig=PlatformConfigStub(extra={"base_url": ""}))
+        assert cfg.base_url.explicit is False
+
+    def test_default_value_is_not_explicit(self, monkeypatch):
+        monkeypatch.delenv("CHATTO_BASE_URL", raising=False)
+        cfg = ChattoConfiguration(pconfig=PlatformConfigStub(extra={}))
+        assert cfg.base_url.explicit is False
+
+    def test_user_set_default_value_is_explicit(self, monkeypatch):
+        """A user may deliberately set base_url to the default string — the
+        value alone cannot say so, which is what this flag records."""
+        monkeypatch.delenv("CHATTO_BASE_URL", raising=False)
+        cfg = ChattoConfiguration(
+            pconfig=PlatformConfigStub(
+                extra={"base_url": ChattoConfiguration.base_url.default}
+            )
+        )
+        assert cfg.base_url.explicit is True
+
+    def test_explicit_false_bool_is_explicit(self, monkeypatch):
+        """`false` is a real user choice, not a blank: the var being present is
+        what records it, since the value alone says nothing."""
+        monkeypatch.setenv("CHATTO_ALLOW_ALL_USERS", "false")
+        cfg = ChattoConfiguration(pconfig=PlatformConfigStub(extra={}))
+        assert cfg.allow_all_users.value is False
+        assert cfg.allow_all_users.explicit is True
+
+
+class TestProfileSecretOverlay:
+    """The tool layer re-reads the profile's own .env for values the process
+    environment no longer carries — explicit user input must survive that."""
+
+    def test_default_yields_to_a_profile_secret(self):
+        cfg = ChattoConfiguration(pconfig=PlatformConfigStub(extra={}))
+        cfg.overlay_profile_secrets({"CHATTO_TOKEN": "from-profile"})
+        assert cfg.token.value == "from-profile"
+
+    def test_explicit_value_survives_the_overlay(self):
+        cfg = ChattoConfiguration(
+            pconfig=PlatformConfigStub(extra={"token": "from-config-yaml"})
+        )
+        cfg.overlay_profile_secrets({"CHATTO_TOKEN": "from-profile"})
+        assert cfg.token.value == "from-config-yaml"
+
+    def test_blank_placeholder_yields_to_a_profile_secret(self):
+        """`CHATTO_TOKEN=` left behind by the installer is not intent."""
+        cfg = ChattoConfiguration(pconfig=PlatformConfigStub(extra={"token": ""}))
+        cfg.overlay_profile_secrets({"CHATTO_TOKEN": "from-profile"})
+        assert cfg.token.value == "from-profile"
+
+    def test_absent_profile_secret_leaves_the_value_alone(self):
+        cfg = ChattoConfiguration(pconfig=PlatformConfigStub(extra={}))
+        cfg.overlay_profile_secrets({})
+        assert cfg.token.value is None
+
+
 class PlatformConfigStub:
     """Just the attribute ChattoConfiguration reads off a PlatformConfig."""
 

Some files were not shown because too many files changed in this diff