Merge pull request #77 from Point72/tkp/composable-backends

Address backend config review feedback

Author: timkpaine

csp_bot/commands/echo.py

@@ -36,7 +36,7 @@ def execute(self, command: BotCommand) -> Optional[Message]:
 
         # Add mentions for any tagged users
         if command.targets:
-            mentions = mention_users([t.to_chatom_user() for t in command.targets], command.backend)
+            mentions = mention_users(list(command.targets), command.backend)
             if mentions:
                 content = f"{content} {mentions}".strip()
 

csp_bot/config/backend/discord.yaml

@@ -5,3 +5,9 @@ discord:
   config:
     _target_: chatom.discord.DiscordConfig
     token: ${oc.env:DISCORD_TOKEN}
+    # message_content is a privileged intent and must also be enabled in the
+    # Discord Developer Portal for the bot to read command text in guilds.
+    intents:
+      - guilds
+      - messages
+      - message_content

csp_bot/config/backend/symphony.yaml

@@ -4,4 +4,6 @@ symphony:
   bot_name: ${bot_name}
   config:
     _target_: chatom.symphony.SymphonyConfig
-    bot_private_key_content: ${oc.env:SYMPHONY_BOT_KEY}
+    host: ${oc.env:SYMPHONY_HOST}
+    bot_username: ${oc.env:SYMPHONY_BOT_USERNAME}
+    bot_certificate_path: ${oc.env:SYMPHONY_CERT_PATH}

csp_bot/tests/test_config_composition.py

@@ -0,0 +1,61 @@
+"""Tests that the packaged gateway/backend config groups compose correctly."""
+
+import os
+
+import pytest
+from hydra import compose, initialize_config_dir
+from omegaconf import OmegaConf
+
+import csp_bot.config
+
+CONFIG_DIR = os.path.dirname(csp_bot.config.__file__)
+
+GATEWAY_BACKENDS = {
+    "slack": {"slack"},
+    "discord": {"discord"},
+    "symphony": {"symphony"},
+    "telegram": {"telegram"},
+    "mixed": {"slack", "discord"},
+    "all": {"slack", "discord", "symphony", "telegram"},
+}
+
+
+@pytest.fixture
+def backend_env(monkeypatch):
+    """Provide dummy values for every backend's credential variables."""
+    for name in (
+        "SLACK_BOT_TOKEN",
+        "SLACK_APP_TOKEN",
+        "DISCORD_TOKEN",
+        "SYMPHONY_HOST",
+        "SYMPHONY_BOT_USERNAME",
+        "SYMPHONY_CERT_PATH",
+        "TELEGRAM_BOT_TOKEN",
+    ):
+        monkeypatch.setenv(name, "dummy")
+
+
+def _compose(overrides):
+    with initialize_config_dir(config_dir=CONFIG_DIR, version_base=None):
+        cfg = compose(config_name="conf", overrides=overrides)
+        return OmegaConf.to_container(cfg.modules.bot.config, resolve=True)
+
+
+@pytest.mark.parametrize("gateway,expected", GATEWAY_BACKENDS.items())
+def test_precanned_gateway_composes(gateway, expected, backend_env):
+    """Each pre-canned gateway resolves to its expected backend set."""
+    config = _compose([f"+gateway={gateway}"])
+    backends = {k for k in config if k != "_target_"}
+    assert backends == expected
+
+
+def test_bare_bot_has_no_backends(backend_env):
+    """The bare `bot` gateway selects no backends on its own."""
+    config = _compose(["+gateway=bot"])
+    assert {k for k in config if k != "_target_"} == set()
+
+
+def test_ad_hoc_backend_selection(backend_env):
+    """Any combination can be assembled from the bare gateway."""
+    config = _compose(["+gateway=bot", "+backend=[slack,telegram]"])
+    assert {k for k in config if k != "_target_"} == {"slack", "telegram"}

csp_bot/tests/test_echo.py

@@ -70,19 +70,11 @@ def test_execute_with_no_args_returns_none(self):
         assert result is None
 
     def test_execute_with_targets(self):
-        """Test executing echo with targets.
-
-        Note: The echo command calls to_chatom_user() on targets, but the
-        targets are already chatom.User objects. For this test we use a mock.
-        """
-        from unittest.mock import Mock
-
+        """Test executing echo with targets."""
         cmd = EchoCommand()
         channel = Channel(id="ch1", name="test-channel")
 
-        # Create a mock target that has the to_chatom_user method
-        mock_target = Mock()
-        mock_target.to_chatom_user.return_value = User(id="u123", name="testuser")
+        target = User(id="u123", name="testuser")
 
         bot_cmd = BotCommand(
             backend="slack",
@@ -91,7 +83,7 @@ def test_execute_with_targets(self):
             channel_id=channel.id,
             channel_name=channel.name,
             source=User(id="u1", name="sender"),
-            targets=(mock_target,),
+            targets=(target,),
             variant=CommandVariant.REPLY_TO_OTHER,
             message=None,
         )
@@ -104,19 +96,11 @@ def test_execute_with_targets(self):
         assert "<@u123>" in result.content or "testuser" in result.content
 
     def test_execute_with_only_targets(self):
-        """Test executing echo with only targets (no args).
-
-        Note: The echo command calls to_chatom_user() on targets, but the
-        targets are already chatom.User objects. For this test we use a mock.
-        """
-        from unittest.mock import Mock
-
+        """Test executing echo with only targets (no args)."""
         cmd = EchoCommand()
         channel = Channel(id="ch1", name="test-channel")
 
-        # Create a mock target that has the to_chatom_user method
-        mock_target = Mock()
-        mock_target.to_chatom_user.return_value = User(id="u123", name="testuser")
+        target = User(id="u123", name="testuser")
 
         bot_cmd = BotCommand(
             backend="slack",
@@ -125,7 +109,7 @@ def test_execute_with_only_targets(self):
             channel_id=channel.id,
             channel_name=channel.name,
             source=User(id="u1", name="sender"),
-            targets=(mock_target,),
+            targets=(target,),
             variant=CommandVariant.REPLY_TO_OTHER,
             message=None,
         )

docs/wiki/Backends.md

@@ -20,21 +20,26 @@ pip install csp-adapter-slack csp-adapter-telegram
 
 Each backend reads its credentials from environment variables, matching the names used by the built-in `backend` configs.
 
-| Backend  | Environment variables                | `chatom` config field     |
-| :------- | :----------------------------------- | :------------------------ |
-| Slack    | `SLACK_BOT_TOKEN`, `SLACK_APP_TOKEN` | `bot_token`, `app_token`  |
-| Discord  | `DISCORD_TOKEN`                      | `token`                   |
-| Symphony | `SYMPHONY_BOT_KEY`                   | `bot_private_key_content` |
-| Telegram | `TELEGRAM_BOT_TOKEN`                 | `bot_token`               |
+| Backend  | Environment variables                                          | `chatom` config field                          |
+| :------- | :------------------------------------------------------------- | :--------------------------------------------- |
+| Slack    | `SLACK_BOT_TOKEN`, `SLACK_APP_TOKEN`                           | `bot_token`, `app_token`                       |
+| Discord  | `DISCORD_TOKEN`                                                | `token`                                        |
+| Symphony | `SYMPHONY_HOST`, `SYMPHONY_BOT_USERNAME`, `SYMPHONY_CERT_PATH` | `host`, `bot_username`, `bot_certificate_path` |
+| Telegram | `TELEGRAM_BOT_TOKEN`                                           | `bot_token`                                    |
 
-To set a field other than the default (for example, a Symphony key on disk rather than in the environment), override it in your own config:
+Symphony needs a host, a bot username, and either a certificate or an RSA private key.
+The built-in preset uses certificate authentication with a combined certificate/key `.pem` file on disk (`bot_certificate_path`); a path keeps long-lived key material out of the process environment.
+
+Discord's `message_content` is a [privileged intent](https://discord.com/developers/docs/topics/gateway#privileged-intents).
+The built-in preset requests it, but it must also be enabled for the bot in the Discord Developer Portal, or the bot will not receive command text in guild channels.
+
+To set a field other than the defaults — for example, to authenticate Symphony with an RSA key instead of a certificate — override it in your own config:
 
 ```yaml
 # @package modules.bot.config
 symphony:
   config:
     bot_private_key_path: /path/to/bot-key.pem
-    bot_certificate_path: /path/to/bot-cert.pem
 ```
 
 For platform-specific setup of tokens and bot accounts, follow the adapter guides:

docs/wiki/Writing-Commands.md

@@ -32,7 +32,7 @@ class HelloCommand(ReplyToOtherCommand):
         if not command.targets:
             return None
         mentions = mention_users(
-            [t.to_chatom_user() for t in command.targets],
+            list(command.targets),
             command.backend,
         )
         return Message(
@@ -51,7 +51,9 @@ class HelloCommandModel(BaseCommandModel):
 ## Register the command
 
 Commands are selected by configuration.
-Put `hello.py` next to a bot config and list the command alongside the built-ins:
+`csp-bot` imports each command by its `_target_`, so `hello.py` must be importable on the `PYTHONPATH` (for example, run from the directory that contains it, or ship it as part of an installed package).
+
+List your command alongside the built-ins:
 
 **my_bot/bot/slack.yaml**
 
@@ -67,13 +69,16 @@ gateway:
   commands:
     - /commands/help
     - /commands/echo
+    - /commands/schedule
+    - /commands/status
     - _target_: hello.HelloCommandModel
 ```
 
-Then start the bot, pointing Hydra at your config directory so it can import `hello.py`:
+`gateway.commands` replaces the default command list, so include the built-ins you want to keep.
+Then start the bot, adding the config directory to both Hydra's search path and Python's import path:
 
 ```bash
-csp-bot-start --config-dir=my_bot +bot=slack
+PYTHONPATH=my_bot/bot csp-bot-start --config-dir=my_bot +bot=slack
 ```
 
 Tagging the bot with `/hello @someone` now replies with a greeting.

example/bot/all.yaml

@@ -6,5 +6,6 @@ defaults:
 bot_name: CSP Bot
 
 # Tokens are read from the environment:
-#   SLACK_BOT_TOKEN, SLACK_APP_TOKEN, DISCORD_TOKEN, SYMPHONY_BOT_KEY, TELEGRAM_BOT_TOKEN
+#   SLACK_BOT_TOKEN, SLACK_APP_TOKEN, DISCORD_TOKEN,
+#   SYMPHONY_HOST, SYMPHONY_BOT_USERNAME, SYMPHONY_CERT_PATH, TELEGRAM_BOT_TOKEN
 # csp-bot-start --config-dir=example +bot=all

example/bot/symphony.yaml

@@ -5,5 +5,5 @@ defaults:
 
 bot_name: CSP Bot
 
-# Tokens are read from the environment: SYMPHONY_BOT_KEY
+# Credentials are read from the environment: SYMPHONY_HOST, SYMPHONY_BOT_USERNAME, SYMPHONY_CERT_PATH
 # csp-bot-start --config-dir=example +bot=symphony