diff --git a/gateway/session.py b/gateway/session.py index dcc737bbd7..8044c5c2c6 100644 --- a/gateway/session.py +++ b/gateway/session.py @@ -202,6 +202,31 @@ that requires raw IDs). Discord is excluded because mentions use ``<@user_id>`` and the LLM needs the real ID to tag users.""" +def _discord_tools_loaded() -> bool: + """True iff the agent will actually have Discord tools this session. + + Two conditions must hold: + 1. The `discord` or `discord_admin` toolset is enabled for the + Discord platform via `hermes tools` (opt-in, default OFF). + 2. `DISCORD_BOT_TOKEN` is set โ€” the tool's `check_fn` gates on it + at registry time, so the toolset being enabled in config is not + enough if the token isn't configured. + + Returns False (safe default โ€” keeps the stale-API disclaimer) on any + error so a bad config can't silently promise tools the agent lacks. + """ + if not (os.environ.get("DISCORD_BOT_TOKEN") or "").strip(): + return False + try: + from hermes_cli.config import load_config + from hermes_cli.tools_config import _get_platform_tools + cfg = load_config() + enabled = _get_platform_tools(cfg, "discord", include_default_mcp_servers=False) + return "discord" in enabled or "discord_admin" in enabled + except Exception: + return False + + def build_session_context_prompt( context: SessionContext, *, @@ -289,13 +314,12 @@ def build_session_context_prompt( "that you can only read messages sent directly to you and respond." ) elif context.source.platform == Platform.DISCORD: - # The discord tool self-gates on DISCORD_BOT_TOKEN at registry - # check time. Match that condition so the prompt stays honest: - # with a token the agent has fetch_messages/search_members/ - # create_thread (and optionally discord_admin) and should know - # the IDs it can call them with; without one it really is - # limited to reading/replying via the gateway. - if (os.environ.get("DISCORD_BOT_TOKEN") or "").strip(): + # Inject the Discord IDs block only when the agent actually has + # Discord tools loaded this session โ€” i.e. the user opted into + # `discord` / `discord_admin` via `hermes tools` AND the bot + # token is configured. Otherwise keep the stale-API disclaimer + # honest so we never promise tools the agent lacks. + if _discord_tools_loaded(): src = context.source id_lines = ["", "**Discord IDs (for the `discord` / `discord_admin` tools):**"] if src.guild_id: diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 93e9dea132..02fabd45a1 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -68,13 +68,35 @@ CONFIGURABLE_TOOLSETS = [ ("rl", "๐Ÿงช RL Training", "Tinker-Atropos training tools"), ("homeassistant", "๐Ÿ  Home Assistant", "smart home device control"), ("spotify", "๐ŸŽต Spotify", "playback, search, playlists, library"), + ("discord", "๐Ÿ’ฌ Discord (read/participate)", "fetch messages, search members, create thread"), ("discord_admin", "๐Ÿ›ก๏ธ Discord Server Admin", "list channels/roles, pin, assign roles"), ] # Toolsets that are OFF by default for new installs. # They're still in _HERMES_CORE_TOOLS (available at runtime if enabled), # but the setup checklist won't pre-select them for first-time users. -_DEFAULT_OFF_TOOLSETS = {"moa", "homeassistant", "rl", "spotify", "discord_admin"} +_DEFAULT_OFF_TOOLSETS = {"moa", "homeassistant", "rl", "spotify", "discord", "discord_admin"} + +# Platform-scoped toolsets: only appear in the `hermes tools` checklist for +# these platforms, and only resolve/save for these platforms. A toolset +# absent from this map is available on every platform (current behaviour). +# +# Use this for tools whose APIs only make sense on one platform (Discord +# server admin, Slack workspace admin, etc.). Keeps every other platform's +# checklist from filling up with irrelevant toggles. +_TOOLSET_PLATFORM_RESTRICTIONS: Dict[str, Set[str]] = { + "discord": {"discord"}, + "discord_admin": {"discord"}, +} + + +def _toolset_allowed_for_platform(ts_key: str, platform: str) -> bool: + """Return True if ``ts_key`` is configurable on ``platform``. + + Toolsets without a restriction entry are allowed everywhere (the default). + """ + allowed = _TOOLSET_PLATFORM_RESTRICTIONS.get(ts_key) + return allowed is None or platform in allowed def _get_effective_configurable_toolsets(): @@ -617,7 +639,10 @@ def _get_platform_tools( has_explicit_config = any(ts in configurable_keys for ts in toolset_names) if has_explicit_config: - enabled_toolsets = {ts for ts in toolset_names if ts in configurable_keys} + enabled_toolsets = { + ts for ts in toolset_names + if ts in configurable_keys and _toolset_allowed_for_platform(ts, platform) + } else: # No explicit config โ€” fall back to resolving composite toolset names # (e.g. "hermes-cli") to individual tool names and reverse-mapping. @@ -627,12 +652,19 @@ def _get_platform_tools( enabled_toolsets = set() for ts_key, _, _ in CONFIGURABLE_TOOLSETS: + if not _toolset_allowed_for_platform(ts_key, platform): + continue ts_tools = set(resolve_toolset(ts_key)) if ts_tools and ts_tools.issubset(all_tool_names): enabled_toolsets.add(ts_key) default_off = set(_DEFAULT_OFF_TOOLSETS) - if platform in default_off: + # Legacy safety: if the platform's own name matches a default-off + # toolset (e.g. `homeassistant` platform + `homeassistant` toolset), + # keep that toolset enabled on first install. Skip this dodge for + # platform-restricted toolsets โ€” those are always opt-in even on + # their own platform (e.g. `discord` + `discord` should stay OFF). + if platform in default_off and platform not in _TOOLSET_PLATFORM_RESTRICTIONS: default_off.remove(platform) enabled_toolsets -= default_off @@ -735,6 +767,14 @@ def _save_platform_tools(config: dict, platform: str, enabled_toolset_keys: Set[ """ config.setdefault("platform_toolsets", {}) + # Drop platform-scoped toolsets that don't apply here. Prevents the + # "Configure all platforms" checklist (or a hand-edited config.yaml) + # from turning on, say, the `discord` toolset for Telegram. + enabled_toolset_keys = { + ts for ts in enabled_toolset_keys + if _toolset_allowed_for_platform(ts, platform) + } + # Get the set of all configurable toolset keys (built-in + plugin) configurable_keys = {ts_key for ts_key, _, _ in CONFIGURABLE_TOOLSETS} plugin_keys = _get_plugin_toolset_keys() @@ -869,7 +909,7 @@ def _estimate_tool_tokens() -> Dict[str, int]: return _tool_token_cache -def _prompt_toolset_checklist(platform_label: str, enabled: Set[str]) -> Set[str]: +def _prompt_toolset_checklist(platform_label: str, enabled: Set[str], platform: str = "cli") -> Set[str]: """Multi-select checklist of toolsets. Returns set of selected toolset keys.""" from hermes_cli.curses_ui import curses_checklist from toolsets import resolve_toolset @@ -877,7 +917,12 @@ def _prompt_toolset_checklist(platform_label: str, enabled: Set[str]) -> Set[str # Pre-compute per-tool token counts (cached after first call). tool_tokens = _estimate_tool_tokens() - effective = _get_effective_configurable_toolsets() + effective_all = _get_effective_configurable_toolsets() + # Drop platform-scoped toolsets that don't apply to this platform. + effective = [ + (k, l, d) for (k, l, d) in effective_all + if _toolset_allowed_for_platform(k, platform) + ] labels = [] for ts_key, ts_label, ts_desc in effective: @@ -1791,7 +1836,7 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): checklist_preselected = current_enabled - _DEFAULT_OFF_TOOLSETS # Show checklist - new_enabled = _prompt_toolset_checklist(pinfo["label"], checklist_preselected) + new_enabled = _prompt_toolset_checklist(pinfo["label"], checklist_preselected, pkey) added = new_enabled - current_enabled removed = current_enabled - new_enabled @@ -2147,7 +2192,11 @@ def _apply_mcp_change(config: dict, targets: List[str], action: str) -> Set[str] def _print_tools_list(enabled_toolsets: set, mcp_servers: dict, platform: str = "cli"): """Print a summary of enabled/disabled toolsets and MCP tool filters.""" - effective = _get_effective_configurable_toolsets() + effective_all = _get_effective_configurable_toolsets() + effective = [ + (k, l, d) for (k, l, d) in effective_all + if _toolset_allowed_for_platform(k, platform) + ] builtin_keys = {ts_key for ts_key, _, _ in CONFIGURABLE_TOOLSETS} print(f"Built-in toolsets ({platform}):") @@ -2213,6 +2262,20 @@ def tools_disable_enable_command(args): _print_error(f"Unknown toolset '{name}'") toolset_targets = [t for t in toolset_targets if t in valid_toolsets] + # Reject platform-scoped toolsets on platforms that don't allow them. + restricted_targets = [ + t for t in toolset_targets + if not _toolset_allowed_for_platform(t, platform) + ] + if restricted_targets: + for name in restricted_targets: + allowed = sorted(_TOOLSET_PLATFORM_RESTRICTIONS.get(name) or set()) + _print_error( + f"Toolset '{name}' is not available on platform '{platform}' " + f"(only: {', '.join(allowed)})" + ) + toolset_targets = [t for t in toolset_targets if t not in restricted_targets] + if toolset_targets: _apply_toolset_change(config, platform, toolset_targets, action) diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index dcfb258f56..eb31fd5ff3 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -699,18 +699,60 @@ def test_get_platform_tools_second_pass_skips_fully_claimed_toolsets(): assert "search" not in enabled -def test_get_platform_tools_discord_includes_discord_not_admin(): +def test_get_platform_tools_discord_both_off_by_default(): + """Both `discord` and `discord_admin` are opt-in via `hermes tools`, + even on the Discord platform itself. Users shouldn't auto-inherit 19 + extra tools just because DISCORD_BOT_TOKEN is set.""" enabled = _get_platform_tools({}, "discord") + assert "discord" not in enabled + assert "discord_admin" not in enabled + + +def test_discord_toolsets_in_configurable_toolsets(): + keys = {ts_key for ts_key, _, _ in CONFIGURABLE_TOOLSETS} + assert "discord" in keys + assert "discord_admin" in keys + + +def test_discord_toolsets_in_default_off(): + assert "discord" in _DEFAULT_OFF_TOOLSETS + assert "discord_admin" in _DEFAULT_OFF_TOOLSETS + + +def test_discord_toolsets_not_available_on_other_platforms(): + """Platform-scoping: discord / discord_admin should not appear on CLI, + Telegram, etc. โ€” not even as an opt-in.""" + from hermes_cli.tools_config import _toolset_allowed_for_platform + for plat in ["cli", "telegram", "slack", "whatsapp", "signal"]: + assert not _toolset_allowed_for_platform("discord", plat), ( + f"`discord` toolset leaked onto {plat}" + ) + assert not _toolset_allowed_for_platform("discord_admin", plat), ( + f"`discord_admin` toolset leaked onto {plat}" + ) + assert _toolset_allowed_for_platform("discord", "discord") + assert _toolset_allowed_for_platform("discord_admin", "discord") + + +def test_discord_toolsets_user_enabled_are_honored(): + """When the user opts in via `hermes tools`, the toolset appears.""" + config = {"platform_toolsets": {"discord": ["web", "terminal", "discord"]}} + enabled = _get_platform_tools(config, "discord") assert "discord" in enabled assert "discord_admin" not in enabled -def test_discord_admin_in_configurable_toolsets(): - assert any(ts_key == "discord_admin" for ts_key, _, _ in CONFIGURABLE_TOOLSETS) - - -def test_discord_admin_in_default_off(): - assert "discord_admin" in _DEFAULT_OFF_TOOLSETS +def test_save_platform_tools_strips_restricted_toolsets(): + """Hand-edited or all-platforms checklist with `discord` selected for + Telegram must be stripped at save time.""" + from hermes_cli.tools_config import _save_platform_tools + config = {} + _save_platform_tools(config, "telegram", {"web", "terminal", "discord", "discord_admin"}) + saved = config["platform_toolsets"]["telegram"] + assert "discord" not in saved + assert "discord_admin" not in saved + assert "web" in saved + assert "terminal" in saved def test_get_platform_tools_feishu_includes_doc_and_drive():