mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
fix(cli): expand composite toolset when mixed with configurables in platform_toolsets
When platform_toolsets[<platform>] 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.
This commit is contained in:
parent
1f4200debf
commit
7d276bfbee
2 changed files with 90 additions and 0 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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": []}}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue