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.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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))),
|
||||
|
||||
Reference in New Issue
Block a user