From 62f8aa9b03ef2db41f1dddf356e8142a26553658 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 26 Mar 2026 13:39:41 -0700 Subject: [PATCH] fix: MCP toolset resolution for runtime and config (#3252) Gateway sessions had their own inline toolset resolution that only read platform_toolsets from config, which never includes MCP server names. MCP tools were discovered and registered but invisible to the model. - Replace duplicated gateway toolset resolution in _run_agent() and _run_background_task() with calls to the shared _get_platform_tools() - Extend _get_platform_tools() to include globally enabled MCP servers at runtime (include_default_mcp_servers=True), while config-editing flows use include_default_mcp_servers=False to avoid persisting implicit MCP defaults into platform_toolsets - Add homeassistant to PLATFORMS dict (was missing, caused KeyError) - Fix CLI entry point to use _get_platform_tools() as well, so MCP tools are visible in CLI mode too - Remove redundant platform_key reassignment in _run_background_task Co-authored-by: kshitijk4poor --- cli.py | 9 +- gateway/run.py | 152 +++++------------- hermes_cli/tools_config.py | 74 +++++++-- tests/gateway/test_reasoning_command.py | 109 +++++++++++++ tests/hermes_cli/test_tools_config.py | 33 ++++ tests/hermes_cli/test_tools_disable_enable.py | 17 ++ 6 files changed, 262 insertions(+), 132 deletions(-) diff --git a/cli.py b/cli.py index 40751274e..6475db00e 100644 --- a/cli.py +++ b/cli.py @@ -7288,12 +7288,9 @@ def main( else: toolsets_list.append(str(t)) else: - # Check config for CLI toolsets, fallback to hermes-cli - config_cli_toolsets = CLI_CONFIG.get("platform_toolsets", {}).get("cli") - if config_cli_toolsets and isinstance(config_cli_toolsets, list): - toolsets_list = config_cli_toolsets - else: - toolsets_list = ["hermes-cli"] + # Use the shared resolver so MCP servers are included at runtime + from hermes_cli.tools_config import _get_platform_tools + toolsets_list = sorted(_get_platform_tools(CLI_CONFIG, "cli")) parsed_skills = _parse_skills_argument(skills) diff --git a/gateway/run.py b/gateway/run.py index 2d58363e1..d56db228a 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -257,7 +257,25 @@ def _resolve_runtime_agent_kwargs() -> dict: } -def _resolve_gateway_model() -> str: +def _platform_config_key(platform: "Platform") -> str: + """Map a Platform enum to its config.yaml key (LOCALβ†’"cli", restβ†’enum value).""" + return "cli" if platform == Platform.LOCAL else platform.value + + +def _load_gateway_config() -> dict: + """Load and parse ~/.hermes/config.yaml, returning {} on any error.""" + try: + config_path = _hermes_home / 'config.yaml' + if config_path.exists(): + import yaml + with open(config_path, 'r', encoding='utf-8') as f: + return yaml.safe_load(f) or {} + except Exception: + logger.debug("Could not load gateway config from %s", _hermes_home / 'config.yaml') + return {} + + +def _resolve_gateway_model(config: dict | None = None) -> str: """Read model from env/config β€” mirrors the resolution in _run_agent_sync. Without this, temporary AIAgent instances (memory flush, /compress) fall @@ -265,19 +283,12 @@ def _resolve_gateway_model() -> str: when the active provider is openai-codex. """ model = os.getenv("HERMES_MODEL") or os.getenv("LLM_MODEL") or "anthropic/claude-opus-4.6" - try: - import yaml as _y - _cfg_path = _hermes_home / "config.yaml" - if _cfg_path.exists(): - with open(_cfg_path, encoding="utf-8") as _f: - _cfg = _y.safe_load(_f) or {} - _model_cfg = _cfg.get("model", {}) - if isinstance(_model_cfg, str): - model = _model_cfg - elif isinstance(_model_cfg, dict): - model = _model_cfg.get("default", model) - except Exception: - pass + cfg = config if config is not None else _load_gateway_config() + model_cfg = cfg.get("model", {}) + if isinstance(model_cfg, str): + model = model_cfg + elif isinstance(model_cfg, dict): + model = model_cfg.get("default", model) return model @@ -3571,52 +3582,12 @@ class GatewayRunner: ) return - # Read model from config via shared helper - model = _resolve_gateway_model() + user_config = _load_gateway_config() + model = _resolve_gateway_model(user_config) + platform_key = _platform_config_key(source.platform) - # Determine toolset (same logic as _run_agent) - default_toolset_map = { - Platform.LOCAL: "hermes-cli", - Platform.TELEGRAM: "hermes-telegram", - Platform.DISCORD: "hermes-discord", - Platform.WHATSAPP: "hermes-whatsapp", - Platform.SLACK: "hermes-slack", - Platform.SIGNAL: "hermes-signal", - Platform.HOMEASSISTANT: "hermes-homeassistant", - Platform.EMAIL: "hermes-email", - Platform.DINGTALK: "hermes-dingtalk", - } - platform_toolsets_config = {} - try: - config_path = _hermes_home / 'config.yaml' - if config_path.exists(): - import yaml - with open(config_path, 'r', encoding="utf-8") as f: - user_config = yaml.safe_load(f) or {} - platform_toolsets_config = user_config.get("platform_toolsets", {}) - except Exception: - pass - - platform_config_key = { - Platform.LOCAL: "cli", - Platform.TELEGRAM: "telegram", - Platform.DISCORD: "discord", - Platform.WHATSAPP: "whatsapp", - Platform.SLACK: "slack", - Platform.SIGNAL: "signal", - Platform.HOMEASSISTANT: "homeassistant", - Platform.EMAIL: "email", - Platform.DINGTALK: "dingtalk", - }.get(source.platform, "telegram") - - config_toolsets = platform_toolsets_config.get(platform_config_key) - if config_toolsets and isinstance(config_toolsets, list): - enabled_toolsets = config_toolsets - else: - default_toolset = default_toolset_map.get(source.platform, "hermes-telegram") - enabled_toolsets = [default_toolset] - - platform_key = "cli" if source.platform == Platform.LOCAL else source.platform.value + from hermes_cli.tools_config import _get_platform_tools + enabled_toolsets = sorted(_get_platform_tools(user_config, platform_key)) pr = self._provider_routing max_iterations = int(os.getenv("HERMES_MAX_ITERATIONS", "90")) @@ -4742,67 +4713,16 @@ class GatewayRunner: from run_agent import AIAgent import queue - # Determine toolset based on platform. - # Check config.yaml for per-platform overrides, fallback to hardcoded defaults. - default_toolset_map = { - Platform.LOCAL: "hermes-cli", - Platform.TELEGRAM: "hermes-telegram", - Platform.DISCORD: "hermes-discord", - Platform.WHATSAPP: "hermes-whatsapp", - Platform.SLACK: "hermes-slack", - Platform.SIGNAL: "hermes-signal", - Platform.HOMEASSISTANT: "hermes-homeassistant", - Platform.EMAIL: "hermes-email", - Platform.DINGTALK: "hermes-dingtalk", - } + user_config = _load_gateway_config() + platform_key = _platform_config_key(source.platform) - # Try to load platform_toolsets from config - platform_toolsets_config = {} - try: - config_path = _hermes_home / 'config.yaml' - if config_path.exists(): - import yaml - with open(config_path, 'r', encoding="utf-8") as f: - user_config = yaml.safe_load(f) or {} - platform_toolsets_config = user_config.get("platform_toolsets", {}) - except Exception as e: - logger.debug("Could not load platform_toolsets config: %s", e) + from hermes_cli.tools_config import _get_platform_tools + enabled_toolsets = sorted(_get_platform_tools(user_config, platform_key)) - # Map platform enum to config key - platform_config_key = { - Platform.LOCAL: "cli", - Platform.TELEGRAM: "telegram", - Platform.DISCORD: "discord", - Platform.WHATSAPP: "whatsapp", - Platform.SLACK: "slack", - Platform.SIGNAL: "signal", - Platform.HOMEASSISTANT: "homeassistant", - Platform.EMAIL: "email", - Platform.DINGTALK: "dingtalk", - }.get(source.platform, "telegram") - - # Use config override if present (list of toolsets), otherwise hardcoded default - config_toolsets = platform_toolsets_config.get(platform_config_key) - if config_toolsets and isinstance(config_toolsets, list): - enabled_toolsets = config_toolsets - else: - default_toolset = default_toolset_map.get(source.platform, "hermes-telegram") - enabled_toolsets = [default_toolset] - # Tool progress mode from config.yaml: "all", "new", "verbose", "off" # Falls back to env vars for backward compatibility - _progress_cfg = {} - try: - _tp_cfg_path = _hermes_home / "config.yaml" - if _tp_cfg_path.exists(): - import yaml as _tp_yaml - with open(_tp_cfg_path, encoding="utf-8") as _tp_f: - _tp_data = _tp_yaml.safe_load(_tp_f) or {} - _progress_cfg = _tp_data.get("display", {}) - except Exception: - pass progress_mode = ( - _progress_cfg.get("tool_progress") + user_config.get("display", {}).get("tool_progress") or os.getenv("HERMES_TOOL_PROGRESS_MODE") or "all" ) @@ -5025,7 +4945,7 @@ class GatewayRunner: except Exception: pass - model = _resolve_gateway_model() + model = _resolve_gateway_model(user_config) try: runtime_kwargs = _resolve_runtime_agent_kwargs() diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index a8f349e9c..6f76c98ae 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -131,6 +131,7 @@ PLATFORMS = { "slack": {"label": "πŸ’Ό Slack", "default_toolset": "hermes-slack"}, "whatsapp": {"label": "πŸ“± WhatsApp", "default_toolset": "hermes-whatsapp"}, "signal": {"label": "πŸ“‘ Signal", "default_toolset": "hermes-signal"}, + "homeassistant": {"label": "🏠 Home Assistant", "default_toolset": "hermes-homeassistant"}, "email": {"label": "πŸ“§ Email", "default_toolset": "hermes-email"}, "dingtalk": {"label": "πŸ’¬ DingTalk", "default_toolset": "hermes-dingtalk"}, } @@ -378,7 +379,29 @@ def _platform_toolset_summary(config: dict, platforms: Optional[List[str]] = Non return summary -def _get_platform_tools(config: dict, platform: str) -> Set[str]: +def _parse_enabled_flag(value, default: bool = True) -> bool: + """Parse bool-like config values used by tool/platform settings.""" + if value is None: + return default + if isinstance(value, bool): + return value + if isinstance(value, int): + return value != 0 + if isinstance(value, str): + lowered = value.strip().lower() + if lowered in {"true", "1", "yes", "on"}: + return True + if lowered in {"false", "0", "no", "off"}: + return False + return default + + +def _get_platform_tools( + config: dict, + platform: str, + *, + include_default_mcp_servers: bool = True, +) -> Set[str]: """Resolve which individual toolset names are enabled for a platform.""" from toolsets import resolve_toolset @@ -430,6 +453,37 @@ def _get_platform_tools(config: dict, platform: str) -> Set[str]: enabled_toolsets.add(pts) # else: known but not in config = user disabled it + # Preserve any explicit non-configurable toolset entries (for example, + # custom toolsets or MCP server names saved in platform_toolsets). + platform_default_keys = {p["default_toolset"] for p in PLATFORMS.values()} + explicit_passthrough = { + ts + for ts in toolset_names + if ts not in configurable_keys + and ts not in plugin_ts_keys + and ts not in platform_default_keys + } + + # MCP servers are expected to be available on all platforms by default. + # If the platform explicitly lists one or more MCP server names, treat that + # as an allowlist. Otherwise include every globally enabled MCP server. + mcp_servers = config.get("mcp_servers", {}) + enabled_mcp_servers = { + name + for name, server_cfg in mcp_servers.items() + if isinstance(server_cfg, dict) + and _parse_enabled_flag(server_cfg.get("enabled", True), default=True) + } + explicit_mcp_servers = explicit_passthrough & enabled_mcp_servers + enabled_toolsets.update(explicit_passthrough - enabled_mcp_servers) + if include_default_mcp_servers: + if explicit_mcp_servers: + enabled_toolsets.update(explicit_mcp_servers) + else: + enabled_toolsets.update(enabled_mcp_servers) + else: + enabled_toolsets.update(explicit_mcp_servers) + return enabled_toolsets @@ -1022,7 +1076,7 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): if first_install: for pkey in enabled_platforms: pinfo = PLATFORMS[pkey] - current_enabled = _get_platform_tools(config, pkey) + current_enabled = _get_platform_tools(config, pkey, include_default_mcp_servers=False) # Uncheck toolsets that should be off by default checklist_preselected = current_enabled - _DEFAULT_OFF_TOOLSETS @@ -1074,7 +1128,7 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): platform_keys = [] for pkey in enabled_platforms: pinfo = PLATFORMS[pkey] - current = _get_platform_tools(config, pkey) + current = _get_platform_tools(config, pkey, include_default_mcp_servers=False) count = len(current) total = len(_get_effective_configurable_toolsets()) platform_choices.append(f"Configure {pinfo['label']} ({count}/{total} enabled)") @@ -1121,11 +1175,11 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): # Use the union of all platforms' current tools as the starting state all_current = set() for pk in platform_keys: - all_current |= _get_platform_tools(config, pk) + all_current |= _get_platform_tools(config, pk, include_default_mcp_servers=False) new_enabled = _prompt_toolset_checklist("All platforms", all_current) if new_enabled != all_current: for pk in platform_keys: - prev = _get_platform_tools(config, pk) + prev = _get_platform_tools(config, pk, include_default_mcp_servers=False) added = new_enabled - prev removed = prev - new_enabled pinfo_inner = PLATFORMS[pk] @@ -1147,7 +1201,7 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): print(color(" βœ“ Saved configuration for all platforms", Colors.GREEN)) # Update choice labels for ci, pk in enumerate(platform_keys): - new_count = len(_get_platform_tools(config, pk)) + new_count = len(_get_platform_tools(config, pk, include_default_mcp_servers=False)) total = len(_get_effective_configurable_toolsets()) platform_choices[ci] = f"Configure {PLATFORMS[pk]['label']} ({new_count}/{total} enabled)" else: @@ -1159,7 +1213,7 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): pinfo = PLATFORMS[pkey] # Get current enabled toolsets for this platform - current_enabled = _get_platform_tools(config, pkey) + current_enabled = _get_platform_tools(config, pkey, include_default_mcp_servers=False) # Show checklist new_enabled = _prompt_toolset_checklist(pinfo["label"], current_enabled) @@ -1192,7 +1246,7 @@ def tools_command(args=None, first_install: bool = False, config: dict = None): print() # Update the choice label with new count - new_count = len(_get_platform_tools(config, pkey)) + new_count = len(_get_platform_tools(config, pkey, include_default_mcp_servers=False)) total = len(_get_effective_configurable_toolsets()) platform_choices[idx] = f"Configure {pinfo['label']} ({new_count}/{total} enabled)" @@ -1338,7 +1392,7 @@ def _configure_mcp_tools_interactive(config: dict): def _apply_toolset_change(config: dict, platform: str, toolset_names: List[str], action: str): """Add or remove built-in toolsets for a platform.""" - enabled = _get_platform_tools(config, platform) + enabled = _get_platform_tools(config, platform, include_default_mcp_servers=False) if action == "disable": updated = enabled - set(toolset_names) else: @@ -1424,7 +1478,7 @@ def tools_disable_enable_command(args): return if action == "list": - _print_tools_list(_get_platform_tools(config, platform), + _print_tools_list(_get_platform_tools(config, platform, include_default_mcp_servers=False), config.get("mcp_servers") or {}, platform) return diff --git a/tests/gateway/test_reasoning_command.py b/tests/gateway/test_reasoning_command.py index 745094fe2..cb9e01f11 100644 --- a/tests/gateway/test_reasoning_command.py +++ b/tests/gateway/test_reasoning_command.py @@ -218,3 +218,112 @@ class TestReasoningCommand: assert result["final_response"] == "ok" assert _CapturingAgent.last_init is not None assert _CapturingAgent.last_init["reasoning_config"] == {"enabled": False} + + def test_run_agent_includes_enabled_mcp_servers_in_gateway_toolsets(self, tmp_path, monkeypatch): + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text( + "platform_toolsets:\n" + " cli: [web, memory]\n" + "mcp_servers:\n" + " exa:\n" + " url: https://mcp.exa.ai/mcp\n" + " web-search-prime:\n" + " url: https://api.z.ai/api/mcp/web_search_prime/mcp\n", + encoding="utf-8", + ) + + monkeypatch.setattr(gateway_run, "_hermes_home", hermes_home) + monkeypatch.setattr(gateway_run, "_env_path", hermes_home / ".env") + monkeypatch.setattr(gateway_run, "load_dotenv", lambda *args, **kwargs: None) + monkeypatch.setattr( + gateway_run, + "_resolve_runtime_agent_kwargs", + lambda: { + "provider": "openrouter", + "api_mode": "chat_completions", + "base_url": "https://openrouter.ai/api/v1", + "api_key": "test-key", + }, + ) + fake_run_agent = types.ModuleType("run_agent") + fake_run_agent.AIAgent = _CapturingAgent + monkeypatch.setitem(sys.modules, "run_agent", fake_run_agent) + + _CapturingAgent.last_init = None + runner = _make_runner() + + source = SessionSource( + platform=Platform.LOCAL, + chat_id="cli", + chat_name="CLI", + chat_type="dm", + user_id="user-1", + ) + + result = asyncio.run( + runner._run_agent( + message="ping", + context_prompt="", + history=[], + source=source, + session_id="session-1", + session_key="agent:main:local:dm", + ) + ) + + assert result["final_response"] == "ok" + assert _CapturingAgent.last_init is not None + enabled_toolsets = set(_CapturingAgent.last_init["enabled_toolsets"]) + assert "web" in enabled_toolsets + assert "memory" in enabled_toolsets + assert "exa" in enabled_toolsets + assert "web-search-prime" in enabled_toolsets + + def test_run_agent_homeassistant_uses_default_platform_toolset(self, tmp_path, monkeypatch): + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text("", encoding="utf-8") + + monkeypatch.setattr(gateway_run, "_hermes_home", hermes_home) + monkeypatch.setattr(gateway_run, "_env_path", hermes_home / ".env") + monkeypatch.setattr(gateway_run, "load_dotenv", lambda *args, **kwargs: None) + monkeypatch.setattr( + gateway_run, + "_resolve_runtime_agent_kwargs", + lambda: { + "provider": "openrouter", + "api_mode": "chat_completions", + "base_url": "https://openrouter.ai/api/v1", + "api_key": "test-key", + }, + ) + fake_run_agent = types.ModuleType("run_agent") + fake_run_agent.AIAgent = _CapturingAgent + monkeypatch.setitem(sys.modules, "run_agent", fake_run_agent) + + _CapturingAgent.last_init = None + runner = _make_runner() + + source = SessionSource( + platform=Platform.HOMEASSISTANT, + chat_id="ha", + chat_name="Home Assistant", + chat_type="dm", + user_id="user-1", + ) + + result = asyncio.run( + runner._run_agent( + message="ping", + context_prompt="", + history=[], + source=source, + session_id="session-1", + session_key="agent:main:homeassistant:dm", + ) + ) + + assert result["final_response"] == "ok" + assert _CapturingAgent.last_init is not None + assert "homeassistant" in set(_CapturingAgent.last_init["enabled_toolsets"]) diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index 676305dbd..6af9f6629 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -35,6 +35,39 @@ def test_platform_toolset_summary_uses_explicit_platform_list(): assert summary["cli"] == _get_platform_tools(config, "cli") +def test_get_platform_tools_includes_enabled_mcp_servers_by_default(): + config = { + "mcp_servers": { + "exa": {"url": "https://mcp.exa.ai/mcp"}, + "web-search-prime": {"url": "https://api.z.ai/api/mcp/web_search_prime/mcp"}, + "disabled-server": {"url": "https://example.com/mcp", "enabled": False}, + } + } + + enabled = _get_platform_tools(config, "cli") + + assert "exa" in enabled + assert "web-search-prime" in enabled + assert "disabled-server" not in enabled + + +def test_get_platform_tools_keeps_enabled_mcp_servers_with_explicit_builtin_selection(): + config = { + "platform_toolsets": {"cli": ["web", "memory"]}, + "mcp_servers": { + "exa": {"url": "https://mcp.exa.ai/mcp"}, + "web-search-prime": {"url": "https://api.z.ai/api/mcp/web_search_prime/mcp"}, + }, + } + + enabled = _get_platform_tools(config, "cli") + + assert "web" in enabled + assert "memory" in enabled + assert "exa" in enabled + assert "web-search-prime" in enabled + + def test_toolset_has_keys_for_vision_accepts_codex_auth(tmp_path, monkeypatch): monkeypatch.setenv("HERMES_HOME", str(tmp_path)) (tmp_path / "auth.json").write_text( diff --git a/tests/hermes_cli/test_tools_disable_enable.py b/tests/hermes_cli/test_tools_disable_enable.py index 0976533b1..450f6357a 100644 --- a/tests/hermes_cli/test_tools_disable_enable.py +++ b/tests/hermes_cli/test_tools_disable_enable.py @@ -134,6 +134,23 @@ class TestToolsMixedTargets: assert "web" not in saved["platform_toolsets"]["cli"] assert "create_issue" in saved["mcp_servers"]["github"]["tools"]["exclude"] + def test_builtin_toggle_does_not_persist_implicit_mcp_defaults(self): + config = { + "platform_toolsets": {"cli": ["web", "memory"]}, + "mcp_servers": {"exa": {"url": "https://mcp.exa.ai/mcp"}}, + } + with patch("hermes_cli.tools_config.load_config", return_value=config), \ + patch("hermes_cli.tools_config.save_config") as mock_save: + tools_disable_enable_command(Namespace( + tools_action="disable", + names=["web"], + platform="cli", + )) + saved = mock_save.call_args[0][0] + assert "web" not in saved["platform_toolsets"]["cli"] + assert "memory" in saved["platform_toolsets"]["cli"] + assert "exa" not in saved["platform_toolsets"]["cli"] + # ── List output ──────────────────────────────────────────────────────────────