From 7d276bfbee601f670989ff54c7bd90172af90250 Mon Sep 17 00:00:00 2001 From: JackJin <1037461232@qq.com> Date: Sat, 9 May 2026 22:29:38 +0800 Subject: [PATCH] fix(cli): expand composite toolset when mixed with configurables in platform_toolsets When platform_toolsets[] contains both a composite (e.g. hermes-cli) and at least one configurable opt-in (e.g. spotify), the has_explicit_config branch in _get_platform_tools silently dropped the composite, leaving sessions with only the configurable + plugin tools and no native tools (terminal, file, web, browser, memory, etc.). Mirror the else-branch's subset inference for composites that sit alongside the configurables, but apply _DEFAULT_OFF_TOOLSETS only to the implicit expansion so user-listed default-off toolsets (spotify, discord) survive. --- hermes_cli/tools_config.py | 32 +++++++++++++++ tests/hermes_cli/test_tools_config.py | 58 +++++++++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 152877c2261..7cf90466e07 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -972,6 +972,38 @@ def _get_platform_tools( ts for ts in toolset_names if ts in configurable_keys and _toolset_allowed_for_platform(ts, platform) } + # Mixed config: composite toolset alongside configurables (e.g. + # ``[hermes-cli, spotify]`` after enabling Spotify via ``hermes + # tools``). Without expansion the composite name is silently dropped, + # leaving sessions with only the configurable opt-ins and no native + # tools. Mirror the else-branch's subset inference, but apply + # _DEFAULT_OFF_TOOLSETS only to the implicit expansion — anything the + # user explicitly listed (e.g. ``spotify``) must survive. + composite_tools = set() + for ts_name in toolset_names: + if ts_name in configurable_keys or ts_name in plugin_ts_keys: + continue + if ts_name not in TOOLSETS: + continue + composite_tools.update(resolve_toolset(ts_name)) + + if composite_tools: + expanded = 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(composite_tools): + expanded.add(ts_key) + + default_off = set(_DEFAULT_OFF_TOOLSETS) + if platform in default_off and platform not in _TOOLSET_PLATFORM_RESTRICTIONS: + default_off.remove(platform) + if "homeassistant" in default_off and os.getenv("HASS_TOKEN"): + default_off.remove("homeassistant") + expanded -= default_off + + enabled_toolsets |= expanded else: # No explicit config — fall back to resolving composite toolset names # (e.g. "hermes-cli") to individual tool names and reverse-mapping. diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index 0bde24fc74e..b284d5df199 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -119,6 +119,64 @@ def test_get_platform_tools_homeassistant_toolset_off_for_cron_when_hass_token_m assert "homeassistant" not in cron_enabled +def test_get_platform_tools_expands_composite_when_mixed_with_configurable(): + """``[hermes-cli, spotify]`` (composite + configurable) must keep the full + ``hermes-cli`` toolset alongside the explicit Spotify opt-in. The + has_explicit_config branch used to drop ``hermes-cli`` on the floor, + leaving sessions with only ``{spotify, kanban}``.""" + config = {"platform_toolsets": {"cli": ["hermes-cli", "spotify"]}} + + enabled = _get_platform_tools(config, "cli", include_default_mcp_servers=False) + + # Native tools must reappear. + for ts in ("terminal", "file", "web", "browser", "memory", "delegation", + "code_execution", "todo", "session_search", "skills"): + assert ts in enabled, f"{ts} should be enabled when hermes-cli is listed" + # User explicitly opted into Spotify — must survive _DEFAULT_OFF_TOOLSETS subtraction. + assert "spotify" in enabled + + +def test_get_platform_tools_composite_only_unchanged(): + """Composite-only config (no configurable in list) must still take the + else-branch path and produce the full toolset — guards against the new + code accidentally hijacking the composite-only case.""" + composite_only = _get_platform_tools( + {"platform_toolsets": {"cli": ["hermes-cli"]}}, + "cli", + include_default_mcp_servers=False, + ) + default = _get_platform_tools({}, "cli", include_default_mcp_servers=False) + + assert composite_only == default + + +def test_get_platform_tools_configurable_only_no_expansion(): + """Configurable-only list (no composite) must not pull in unrelated + toolsets — guards against the expansion firing when ``composite_tools`` + is empty.""" + config = {"platform_toolsets": {"cli": ["terminal", "file"]}} + + enabled = _get_platform_tools(config, "cli", include_default_mcp_servers=False) + + assert "terminal" in enabled + assert "file" in enabled + # Web shouldn't sneak in via the new expansion path. + assert "web" not in enabled + + +def test_get_platform_tools_mixed_does_not_resurrect_default_off(): + """Expansion must subtract _DEFAULT_OFF_TOOLSETS from the implicit + pull-in. Without this, ``hermes-cli`` expansion would re-enable + ``moa`` / ``rl`` / ``homeassistant`` for users who never opted in.""" + config = {"platform_toolsets": {"cli": ["hermes-cli", "terminal"]}} + + enabled = _get_platform_tools(config, "cli", include_default_mcp_servers=False) + + assert "terminal" in enabled + assert "moa" not in enabled + assert "rl" not in enabled + + def test_get_platform_tools_preserves_explicit_empty_selection(): config = {"platform_toolsets": {"cli": []}}