mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix: platform default toolsets silently override tool deselection in hermes tools (#2624)
Cherry-picked from PR #2576 by ereid7, plus read-side fix from173a5c62. Both fixes were originally landed in173a5c62but were inadvertently reverted by commit34be3f8b(a squash-merge that bundled unrelated tools_config.py changes). Save side (_save_platform_tools): exclude platform default toolset names (hermes-cli, hermes-telegram) from preserved entries so they don't silently re-enable everything. Read side (_get_platform_tools): when the saved list contains explicit configurable keys, use direct membership instead of subset inference. The subset approach is broken when composite toolsets like hermes-cli resolve to ALL tools.
This commit is contained in:
parent
9d6148316c
commit
868b3c07e3
2 changed files with 134 additions and 13 deletions
|
|
@ -391,18 +391,29 @@ def _get_platform_tools(config: dict, platform: str) -> Set[str]:
|
|||
default_ts = PLATFORMS[platform]["default_toolset"]
|
||||
toolset_names = [default_ts]
|
||||
|
||||
# Resolve to individual tool names, then map back to which
|
||||
# configurable toolsets are covered
|
||||
all_tool_names = set()
|
||||
for ts_name in toolset_names:
|
||||
all_tool_names.update(resolve_toolset(ts_name))
|
||||
configurable_keys = {ts_key for ts_key, _, _ in CONFIGURABLE_TOOLSETS}
|
||||
|
||||
# Map individual tool names back to configurable toolset keys
|
||||
enabled_toolsets = set()
|
||||
for ts_key, _, _ in CONFIGURABLE_TOOLSETS:
|
||||
ts_tools = set(resolve_toolset(ts_key))
|
||||
if ts_tools and ts_tools.issubset(all_tool_names):
|
||||
enabled_toolsets.add(ts_key)
|
||||
# If the saved list contains any configurable keys directly, the user
|
||||
# has explicitly configured this platform — use direct membership.
|
||||
# This avoids the subset-inference bug where composite toolsets like
|
||||
# "hermes-cli" (which include all _HERMES_CORE_TOOLS) cause disabled
|
||||
# toolsets to re-appear as enabled.
|
||||
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}
|
||||
else:
|
||||
# No explicit config — fall back to resolving composite toolset names
|
||||
# (e.g. "hermes-cli") to individual tool names and reverse-mapping.
|
||||
all_tool_names = set()
|
||||
for ts_name in toolset_names:
|
||||
all_tool_names.update(resolve_toolset(ts_name))
|
||||
|
||||
enabled_toolsets = set()
|
||||
for ts_key, _, _ in CONFIGURABLE_TOOLSETS:
|
||||
ts_tools = set(resolve_toolset(ts_key))
|
||||
if ts_tools and ts_tools.issubset(all_tool_names):
|
||||
enabled_toolsets.add(ts_key)
|
||||
|
||||
# Plugin toolsets: enabled by default unless explicitly disabled.
|
||||
# A plugin toolset is "known" for a platform once `hermes tools`
|
||||
|
|
@ -437,15 +448,21 @@ def _save_platform_tools(config: dict, platform: str, enabled_toolset_keys: Set[
|
|||
plugin_keys = _get_plugin_toolset_keys()
|
||||
configurable_keys |= plugin_keys
|
||||
|
||||
# Also exclude platform default toolsets (hermes-cli, hermes-telegram, etc.)
|
||||
# These are "super" toolsets that resolve to ALL tools, so preserving them
|
||||
# would silently override the user's unchecked selections on the next read.
|
||||
platform_default_keys = {p["default_toolset"] for p in PLATFORMS.values()}
|
||||
|
||||
# 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)
|
||||
# Preserve any entries that are NOT configurable toolsets and NOT platform
|
||||
# defaults (i.e. only MCP server names should be preserved)
|
||||
preserved_entries = {
|
||||
entry for entry in existing_toolsets
|
||||
if entry not in configurable_keys
|
||||
if entry not in configurable_keys and entry not in platform_default_keys
|
||||
}
|
||||
|
||||
# Merge preserved entries with new enabled toolsets
|
||||
|
|
|
|||
|
|
@ -100,3 +100,107 @@ def test_save_platform_tools_handles_invalid_existing_config():
|
|||
|
||||
saved_toolsets = config["platform_toolsets"]["cli"]
|
||||
assert "web" in saved_toolsets
|
||||
|
||||
|
||||
def test_save_platform_tools_does_not_preserve_platform_default_toolsets():
|
||||
"""Platform default toolsets (hermes-cli, hermes-telegram, etc.) must NOT
|
||||
be preserved across saves.
|
||||
|
||||
These "super" toolsets resolve to ALL tools, so if they survive in the
|
||||
config, they silently override any tools the user unchecked. Previously,
|
||||
the preserve filter only excluded configurable toolset keys (web, browser,
|
||||
terminal, etc.) and treated platform defaults as unknown custom entries
|
||||
(like MCP server names), causing them to be kept unconditionally.
|
||||
|
||||
Regression test: user unchecks image_gen and homeassistant via
|
||||
``hermes tools``, but hermes-cli stays in the config and re-enables
|
||||
everything on the next read.
|
||||
"""
|
||||
config = {
|
||||
"platform_toolsets": {
|
||||
"cli": [
|
||||
"browser", "clarify", "code_execution", "cronjob",
|
||||
"delegation", "file", "hermes-cli", # <-- the culprit
|
||||
"memory", "session_search", "skills", "terminal",
|
||||
"todo", "tts", "vision", "web",
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
# User unchecks image_gen, homeassistant, moa — keeps the rest
|
||||
new_selection = {
|
||||
"browser", "clarify", "code_execution", "cronjob",
|
||||
"delegation", "file", "memory", "session_search",
|
||||
"skills", "terminal", "todo", "tts", "vision", "web",
|
||||
}
|
||||
|
||||
with patch("hermes_cli.tools_config.save_config"):
|
||||
_save_platform_tools(config, "cli", new_selection)
|
||||
|
||||
saved = config["platform_toolsets"]["cli"]
|
||||
|
||||
# hermes-cli must NOT survive — it's a platform default, not an MCP server
|
||||
assert "hermes-cli" not in saved
|
||||
|
||||
# The individual toolset keys the user selected must be present
|
||||
assert "web" in saved
|
||||
assert "terminal" in saved
|
||||
assert "browser" in saved
|
||||
|
||||
# Tools the user unchecked must NOT be present
|
||||
assert "image_gen" not in saved
|
||||
assert "homeassistant" not in saved
|
||||
assert "moa" not in saved
|
||||
|
||||
|
||||
def test_save_platform_tools_does_not_preserve_hermes_telegram():
|
||||
"""Same bug for Telegram — hermes-telegram must not be preserved."""
|
||||
config = {
|
||||
"platform_toolsets": {
|
||||
"telegram": [
|
||||
"browser", "file", "hermes-telegram", "terminal", "web",
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
new_selection = {"browser", "file", "terminal", "web"}
|
||||
|
||||
with patch("hermes_cli.tools_config.save_config"):
|
||||
_save_platform_tools(config, "telegram", new_selection)
|
||||
|
||||
saved = config["platform_toolsets"]["telegram"]
|
||||
assert "hermes-telegram" not in saved
|
||||
assert "web" in saved
|
||||
|
||||
|
||||
def test_save_platform_tools_still_preserves_mcp_with_platform_default_present():
|
||||
"""MCP server names must still be preserved even when platform defaults
|
||||
are being stripped out."""
|
||||
config = {
|
||||
"platform_toolsets": {
|
||||
"cli": [
|
||||
"web", "terminal", "hermes-cli", "my-mcp-server", "github-tools",
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
new_selection = {"web", "browser"}
|
||||
|
||||
with patch("hermes_cli.tools_config.save_config"):
|
||||
_save_platform_tools(config, "cli", new_selection)
|
||||
|
||||
saved = config["platform_toolsets"]["cli"]
|
||||
|
||||
# MCP servers preserved
|
||||
assert "my-mcp-server" in saved
|
||||
assert "github-tools" in saved
|
||||
|
||||
# Platform default stripped
|
||||
assert "hermes-cli" not in saved
|
||||
|
||||
# User selections present
|
||||
assert "web" in saved
|
||||
assert "browser" in saved
|
||||
|
||||
# Deselected configurable toolset removed
|
||||
assert "terminal" not in saved
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue