From ea25a615a95ea339c4f4fbdc956b219dbc20d7e6 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 19 Sep 2026 22:03:02 -0700 Subject: [PATCH] fix(tools): warn once when tools.tool_search.defer is a scalar, then use the curated default A non-list `defer` value (e.g. `defer: todo_list`) used to be silently ignored - the user tried to shrink the eager tool surface and got nothing back. The parser now logs a warning naming the expected shape (a YAML list of tool names; [] keeps every tool eager) and falls back to the curated default set. Why: config-key fixes must fail closed and loud; #116404 was only noticed because the key was unregistered, and a scalar value would have failed the same silent way. Tests collapsed into one invariant covering the registered default, list/empty-list override, and the scalar warning path. --- tests/tools/test_tool_search.py | 16 ++++++++++------ tools/tool_search.py | 8 ++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/tools/test_tool_search.py b/tests/tools/test_tool_search.py index db8fb9afb4..d5834f068c 100644 --- a/tests/tools/test_tool_search.py +++ b/tests/tools/test_tool_search.py @@ -46,21 +46,25 @@ class TestConfigParsing: assert cfg.enabled == "auto" assert cfg.threshold_pct == 5.0 - def test_default_defer_set_is_shared_with_config_registry(self): + def test_defer_default_is_the_registered_list_and_a_user_list_replaces_it(self, caplog): + """#116404: the curated deferral set lives in DEFAULT_CONFIG (so ``hermes config set`` + recognizes the key); a user list replaces it wholesale, [] keeps every tool eager, and a + scalar is warned about (naming the expected shape) before falling back to the default.""" from hermes_cli.config_defaults import DEFAULT_CONFIG from tools.tool_search import ToolSearchConfig, _DEFAULT_DEFERRED_TOOLS configured = frozenset(DEFAULT_CONFIG["tools"]["tool_search"]["defer"]) - assert configured + assert isinstance(DEFAULT_CONFIG["tools"]["tool_search"]["defer"], list) and configured assert _DEFAULT_DEFERRED_TOOLS == configured assert ToolSearchConfig.from_raw(None).effective_defer_tools == configured - - def test_explicit_defer_list_and_empty_list_override_default(self): - from tools.tool_search import ToolSearchConfig - assert ToolSearchConfig.from_raw({"defer": ["terminal"]}).effective_defer_tools == {"terminal"} assert ToolSearchConfig.from_raw({"defer": []}).effective_defer_tools == set() + with caplog.at_level("WARNING", logger="tools.tool_search"): + assert ToolSearchConfig.from_raw({"defer": "todo_list"}).effective_defer_tools == configured + assert any("tools.tool_search.defer" in r.getMessage() and "expected a YAML list" in r.getMessage() + for r in caplog.records) + def test_bool_true_maps_to_auto(self): from tools.tool_search import ToolSearchConfig cfg = ToolSearchConfig.from_raw(True) diff --git a/tools/tool_search.py b/tools/tool_search.py index 4320770c37..4d953c1a82 100644 --- a/tools/tool_search.py +++ b/tools/tool_search.py @@ -62,6 +62,14 @@ class ToolSearchConfig: raw = {"enabled": "off" if raw is False else "auto"} max_search_limit = _clamped_int(raw.get("max_search_limit"), 25, 1, 50) defer_raw = raw.get("defer") + if defer_raw is not None and not isinstance(defer_raw, (list, tuple, set)): + # Loud, then the curated default: a scalar here means the user tried to shrink the + # tool surface and got nothing — never silently ignore it (#116404). + logger.warning( + "tools.tool_search.defer is %r, expected a YAML list of tool names " + "(e.g. [todo_list, computer_use]; [] keeps every tool eager) - " + "using the curated default set.", defer_raw) + defer_raw = None return cls( enabled=_tri_state(raw.get("enabled", "auto")), threshold_pct=max(0.0, min(100.0, _safe_float(raw.get("threshold_pct"), 5.0))),