From dc77f5c5ed772cf28d995d0cdd8f669b2f947b14 Mon Sep 17 00:00:00 2001 From: robertoetcheverryr <63941860+robertoetcheverryr@users.noreply.github.com> Date: Sat, 17 Oct 2026 13:34:04 -0300 Subject: [PATCH] fix(tools): stop subset toolset 'search' from stripping web_search after Blank Slate (#134593) --- hermes_cli/setup_quick.py | 11 +++- hermes_cli/tools_config.py | 14 +++++- .../test_disabled_toolsets_subset_cleanup.py | 50 +++++++++++++++++++ 3 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 tests/hermes_cli/test_disabled_toolsets_subset_cleanup.py diff --git a/hermes_cli/setup_quick.py b/hermes_cli/setup_quick.py index 3940c6b940..0cb0170f73 100644 --- a/hermes_cli/setup_quick.py +++ b/hermes_cli/setup_quick.py @@ -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: diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index c82e3ccd5f..05d7d95e9b 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -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) diff --git a/tests/hermes_cli/test_disabled_toolsets_subset_cleanup.py b/tests/hermes_cli/test_disabled_toolsets_subset_cleanup.py new file mode 100644 index 0000000000..14dced5943 --- /dev/null +++ b/tests/hermes_cli/test_disabled_toolsets_subset_cleanup.py @@ -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