From 633488e0c08c9bcb1e4c28e744a274d072ac9540 Mon Sep 17 00:00:00 2001 From: ygd58 Date: Sat, 14 Mar 2026 07:58:03 +0100 Subject: [PATCH] fix(tools): preserve MCP toolsets when saving platform tool config _save_platform_tools() overwrote the entire platform_toolsets list with only the toolsets known to CONFIGURABLE_TOOLSETS. This silently dropped any MCP server toolsets that users had added manually to config.yaml. Fix: collect any existing toolset keys that are not in CONFIGURABLE_TOOLSETS and append them back after the wizard's selections are written. This ensures MCP toolsets survive a hermes tools save. Fixes #1247 --- hermes_cli/tools_config.py | 24 +++++++++- tests/hermes_cli/test_tools_config.py | 64 ++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index f12a4b39..b819fafa 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -354,9 +354,29 @@ def _get_platform_tools(config: dict, platform: str) -> Set[str]: def _save_platform_tools(config: dict, platform: str, enabled_toolset_keys: Set[str]): - """Save the selected toolset keys for a platform to config.""" + """Save the selected toolset keys for a platform to config. + + Preserves any non-configurable toolset entries (like MCP server names) + that were already in the config for this platform. + """ config.setdefault("platform_toolsets", {}) - config["platform_toolsets"][platform] = sorted(enabled_toolset_keys) + + # Get the set of all configurable toolset keys + configurable_keys = {ts_key for ts_key, _, _ in CONFIGURABLE_TOOLSETS} + + # Get existing toolsets for this platform + existing_toolsets = config.get("platform_toolsets", {}).get(platform, []) + if not isinstance(existing_toolsets, list): + existing_toolsets = [] + + # Preserve any entries that are NOT configurable toolsets (i.e. MCP server names) + preserved_entries = { + entry for entry in existing_toolsets + if entry not in configurable_keys + } + + # Merge preserved entries with new enabled toolsets + config["platform_toolsets"][platform] = sorted(enabled_toolset_keys | preserved_entries) save_config(config) diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index 92e1e60c..4aee5947 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -1,6 +1,13 @@ """Tests for hermes_cli.tools_config platform tool persistence.""" -from hermes_cli.tools_config import _get_platform_tools, _platform_toolset_summary, _toolset_has_keys +from unittest.mock import patch + +from hermes_cli.tools_config import ( + _get_platform_tools, + _platform_toolset_summary, + _save_platform_tools, + _toolset_has_keys, +) def test_get_platform_tools_uses_default_when_platform_not_configured(): @@ -31,7 +38,7 @@ def test_platform_toolset_summary_uses_explicit_platform_list(): 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( - '{"active_provider":"openai-codex","providers":{"openai-codex":{"tokens":{"access_token":"codex-access-token","refresh_token":"codex-refresh-token"}}}}' + '{"active_provider":"openai-codex","providers":{"openai-codex":{"tokens":{"access_token": "codex-...oken","refresh_token": "codex-...oken"}}}}' ) monkeypatch.delenv("OPENROUTER_API_KEY", raising=False) monkeypatch.delenv("OPENAI_BASE_URL", raising=False) @@ -40,3 +47,56 @@ def test_toolset_has_keys_for_vision_accepts_codex_auth(tmp_path, monkeypatch): monkeypatch.delenv("CONTEXT_VISION_PROVIDER", raising=False) assert _toolset_has_keys("vision") is True + + +def test_save_platform_tools_preserves_mcp_server_names(): + """Ensure MCP server names are preserved when saving platform tools. + + Regression test for https://github.com/NousResearch/hermes-agent/issues/1247 + """ + config = { + "platform_toolsets": { + "cli": ["web", "terminal", "time", "github", "custom-mcp-server"] + } + } + + new_selection = {"web", "browser"} + + with patch("hermes_cli.tools_config.save_config"): + _save_platform_tools(config, "cli", new_selection) + + saved_toolsets = config["platform_toolsets"]["cli"] + + assert "time" in saved_toolsets + assert "github" in saved_toolsets + assert "custom-mcp-server" in saved_toolsets + assert "web" in saved_toolsets + assert "browser" in saved_toolsets + assert "terminal" not in saved_toolsets + + +def test_save_platform_tools_handles_empty_existing_config(): + """Saving platform tools works when no existing config exists.""" + config = {} + + with patch("hermes_cli.tools_config.save_config"): + _save_platform_tools(config, "telegram", {"web", "terminal"}) + + saved_toolsets = config["platform_toolsets"]["telegram"] + assert "web" in saved_toolsets + assert "terminal" in saved_toolsets + + +def test_save_platform_tools_handles_invalid_existing_config(): + """Saving platform tools works when existing config is not a list.""" + config = { + "platform_toolsets": { + "cli": "invalid-string-value" + } + } + + with patch("hermes_cli.tools_config.save_config"): + _save_platform_tools(config, "cli", {"web"}) + + saved_toolsets = config["platform_toolsets"]["cli"] + assert "web" in saved_toolsets