mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(tools): stop subset toolset 'search' from stripping web_search after Blank Slate (#134593)
This commit is contained in:
1 parent
9ee0defd62
commit
dc77f5c5ed
3 files changed
+73
-2
No files matched your search
@@ -210,10 +210,15 @@ def _blank_slate_minimal_toolsets(config: dict):
|
||||
keep = {"file", "terminal", "vision", "skills"}
|
||||
config.setdefault("platform_toolsets", {})["cli"] = sorted(keep)
|
||||
try:
|
||||
from toolsets import TOOLSETS
|
||||
from toolsets import TOOLSETS, resolve_toolset
|
||||
from hermes_cli.tools_config import CONFIGURABLE_TOOLSETS, _get_plugin_toolset_keys
|
||||
all_keys = {k for k, _, _ in CONFIGURABLE_TOOLSETS}
|
||||
all_keys.update(_get_plugin_toolset_keys())
|
||||
# Tools reachable through a checklist row. A plain TOOLSETS entry fully inside one of these (e.g.
|
||||
# `search` = web_search ⊂ `web`) has no row of its own, so disabling it would silently strip those
|
||||
# tools after the user re-enables the superset, with no UI to undo it (#134593).
|
||||
# resolve_toolset returns [] for unknown names and guards cycles, so it needs no try/except.
|
||||
configurable_tool_sets = [set(resolve_toolset(ck)) for ck, _, _ in CONFIGURABLE_TOOLSETS]
|
||||
# Plain TOOLSETS entries catch recovered toolsets like ``kanban``. Skip "hermes-*" platform
|
||||
# composites, "includes" groupings, and posture toolsets (session-level picks by
|
||||
# agent/coding_context.py — disabling them would subtract terminal/read_file).
|
||||
@@ -223,6 +228,10 @@ def _blank_slate_minimal_toolsets(config: dict):
|
||||
# selections made by agent/coding_context.py — not permanent user-facing disables. Adding them
|
||||
# here causes model_tools to subtract their tools (terminal, read_file, …) from the minimal
|
||||
# Blank Slate surface (#57315).
|
||||
if k not in all_keys:
|
||||
tools = set(resolve_toolset(k))
|
||||
if tools and any(tools <= cts for cts in configurable_tool_sets):
|
||||
continue # subset of a checklist toolset — the superset's entry already covers it (#134593)
|
||||
all_keys.add(k)
|
||||
disabled = sorted(all_keys - keep)
|
||||
if disabled:
|
||||
|
||||
@@ -762,7 +762,19 @@ def _save_platform_tools(config: dict, platform: str, enabled_toolset_keys: set[
|
||||
if isinstance(agent_cfg, dict) and agent_cfg.get("disabled_toolsets") and newly_enabled:
|
||||
from agent.skill_utils import parse_config_string_list
|
||||
parsed_disabled = parse_config_string_list(agent_cfg["disabled_toolsets"])
|
||||
remaining = [ts for ts in parsed_disabled if ts not in newly_enabled]
|
||||
# Also clear entries whose tools are entirely covered by a newly enabled toolset: Blank Slate writes
|
||||
# subset toolsets like `search` (only web_search), which have no checklist row, so enabling `web`
|
||||
# left web_search stripped with no supported way to undo it (#134593).
|
||||
from toolsets import resolve_toolset
|
||||
covered: set[str] = set()
|
||||
for ts in newly_enabled:
|
||||
covered.update(resolve_toolset(ts)) # [] for unknown names; never raises
|
||||
|
||||
def _covered(ts: str) -> bool:
|
||||
tools = set(resolve_toolset(ts))
|
||||
return bool(tools) and tools <= covered
|
||||
|
||||
remaining = [ts for ts in parsed_disabled if ts not in newly_enabled and not _covered(ts)]
|
||||
if remaining != parsed_disabled:
|
||||
agent_cfg["disabled_toolsets"] = remaining
|
||||
save_config(config)
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
"""#134593: Blank Slate wrote the subset toolset `search` (only web_search) into agent.disabled_toolsets;
|
||||
enabling `web` via `hermes tools` left it there, silently stripping web_search."""
|
||||
from unittest.mock import patch
|
||||
|
||||
from hermes_cli.setup_quick import _blank_slate_minimal_toolsets
|
||||
from hermes_cli.tools_config import _get_platform_tools, _save_platform_tools
|
||||
from toolsets import resolve_toolset
|
||||
|
||||
|
||||
def _cli_tool_names(config):
|
||||
from model_tools import get_tool_definitions
|
||||
enabled = _get_platform_tools(config, "cli", include_default_mcp_servers=False)
|
||||
return enabled, {t["function"]["name"] for t in get_tool_definitions(enabled_toolsets=sorted(enabled), quiet_mode=True)}
|
||||
|
||||
|
||||
def test_search_is_subset_of_web():
|
||||
assert set(resolve_toolset("search")) <= set(resolve_toolset("web"))
|
||||
|
||||
|
||||
def test_blank_slate_list_excludes_search_but_keeps_kanban():
|
||||
config = {}
|
||||
_blank_slate_minimal_toolsets(config)
|
||||
disabled = config["agent"]["disabled_toolsets"]
|
||||
assert "search" not in disabled
|
||||
assert "kanban" in disabled and "web" in disabled
|
||||
for kept in ("file", "terminal", "vision", "skills"):
|
||||
assert kept not in disabled
|
||||
|
||||
|
||||
def test_enabling_web_clears_legacy_search_entry():
|
||||
# Configs written by Blank Slate before this fix still carry `search`.
|
||||
config = {"platform_toolsets": {"cli": ["file", "terminal"]},
|
||||
"agent": {"disabled_toolsets": ["web", "search", "browser", "memory"]}}
|
||||
with patch("hermes_cli.tools_config.save_config"):
|
||||
_save_platform_tools(config, "cli", {"file", "terminal", "web"})
|
||||
disabled = config["agent"]["disabled_toolsets"]
|
||||
assert "web" not in disabled and "search" not in disabled
|
||||
assert "browser" in disabled and "memory" in disabled
|
||||
|
||||
|
||||
def test_blank_slate_then_enable_web_exposes_web_search():
|
||||
config = {}
|
||||
_blank_slate_minimal_toolsets(config)
|
||||
config["agent"]["disabled_toolsets"].append("search") # legacy Blank Slate output
|
||||
with patch("hermes_cli.tools_config.save_config"):
|
||||
_save_platform_tools(config, "cli", set(config["platform_toolsets"]["cli"]) | {"web"})
|
||||
assert "search" not in config["agent"]["disabled_toolsets"]
|
||||
enabled, names = _cli_tool_names(config)
|
||||
assert "web" in enabled
|
||||
assert {"web_search", "web_extract"} <= names
|
||||
Reference in new issue
Block a user