mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(tools): clear and report legacy disabled_toolsets names like the runtime does
The salvaged coverage check resolved disabled entries with resolve_toolset, which returns nothing for the legacy names the runtime still honours (`terminal_tools`, `web_tools`, ...). So `disabled_toolsets: [terminal_tools]` stripped the terminal tool, `hermes tools list`/doctor told the user to run `hermes tools enable terminal`, and that save left the entry in place forever. _disabled_entry_tools() computes what one entry removes through the runtime's own _apply_toolset_selection (legacy names, bundle/posture non-core delta), and both the save-path coverage check and the repair hint use it. The hint offers `hermes tools enable` only when that save clears every reported entry, else names the entries to remove.
This commit is contained in:
1 parent
1a871b3c5b
commit
b02fa68132
4 files changed
+36
-15
No files matched your search
@@ -507,6 +507,6 @@ def _warn_partially_stripped_toolsets(f: Finding) -> None:
|
||||
from hermes_cli.config import load_config
|
||||
from hermes_cli.tools_config import _get_platform_tools, toolsets_partially_stripped
|
||||
config = load_config() or {}
|
||||
for ts, (tools, by) in toolsets_partially_stripped(config, _get_platform_tools(config, "cli")).items():
|
||||
for ts, (tools, by, hint) in toolsets_partially_stripped(config, _get_platform_tools(config, "cli")).items():
|
||||
check_warn(f"{ts} (partly disabled)", f"({', '.join(tools)} removed by agent.disabled_toolsets: {', '.join(by)})")
|
||||
f.issues.append(f"Run 'hermes tools enable {ts}' to restore {', '.join(tools)}")
|
||||
f.issues.append(f"{ts}: {hint[0].upper()}{hint[1:]} {', '.join(tools)}")
|
||||
@@ -665,11 +665,24 @@ def _prune_toolsets_stripped_by_disabled(enabled_toolsets: set[str], disabled_na
|
||||
return {name for name, tools in resolved.items() if not tools or tools & surviving}
|
||||
|
||||
|
||||
def toolsets_partially_stripped(config: dict, enabled_toolsets: set[str]) -> dict[str, tuple[list[str], list[str]]]:
|
||||
"""``{toolset: (stripped tools, disabled entries stripping them)}`` for enabled toolsets that
|
||||
``agent.disabled_toolsets`` cuts down without removing (``web`` minus ``web_search`` via ``search``,
|
||||
def _disabled_entry_tools(name: str) -> set[str]:
|
||||
"""Tools one ``agent.disabled_toolsets`` entry removes at runtime: legacy names (``terminal_tools``)
|
||||
and the bundle/posture non-core delta included, which ``resolve_toolset`` alone gets wrong."""
|
||||
from model_tools import _LEGACY_TOOLSET_MAP, _apply_toolset_selection
|
||||
from toolsets import resolve_toolset
|
||||
|
||||
universe = set(resolve_toolset(name)) | set(_LEGACY_TOOLSET_MAP.get(name, ()))
|
||||
kept = set(universe)
|
||||
_apply_toolset_selection(kept, [name], quiet_mode=True, disable=True)
|
||||
return universe - kept
|
||||
|
||||
|
||||
def toolsets_partially_stripped(config: dict, enabled_toolsets: set[str]) -> dict[str, tuple[list[str], list[str], str]]:
|
||||
"""``{toolset: (stripped tools, disabled entries stripping them, repair hint)}`` for enabled toolsets
|
||||
that ``agent.disabled_toolsets`` cuts down without removing (``web`` minus ``web_search`` via ``search``,
|
||||
#134593). ``_prune_toolsets_stripped_by_disabled`` keeps such a toolset "enabled" because some of its
|
||||
tools survive, so inspection surfaces need this to say which ones did not."""
|
||||
tools survive, so inspection surfaces need this to say which ones did not. The hint offers
|
||||
``hermes tools enable`` only when that save clears every entry (``_save_platform_tools`` coverage)."""
|
||||
from agent.skill_utils import parse_config_string_list
|
||||
from model_tools import _apply_toolset_selection
|
||||
from toolsets import resolve_toolset, validate_toolset
|
||||
@@ -685,7 +698,11 @@ def toolsets_partially_stripped(config: dict, enabled_toolsets: set[str]) -> dic
|
||||
if tools - kept:
|
||||
by[entry] = tools - kept
|
||||
if by:
|
||||
report[name] = (sorted(set().union(*by.values())), sorted(by))
|
||||
entries = sorted(by)
|
||||
clears = all(_disabled_entry_tools(e) <= tools for e in entries)
|
||||
hint = (f"run `hermes tools enable {name}` to restore" if clears
|
||||
else f"remove {', '.join(entries)} from agent.disabled_toolsets to restore")
|
||||
report[name] = (sorted(set().union(*by.values())), entries, hint)
|
||||
return report
|
||||
|
||||
|
||||
@@ -795,7 +812,7 @@ def _save_platform_tools(config: dict, platform: str, enabled_toolset_keys: set[
|
||||
covered.update(resolve_toolset(ts)) # [] for unknown names; never raises
|
||||
|
||||
def _covered(ts: str) -> bool:
|
||||
tools = set(resolve_toolset(ts))
|
||||
tools = _disabled_entry_tools(ts)
|
||||
return bool(tools) and tools <= covered
|
||||
|
||||
remaining = [ts for ts in parsed_disabled if ts not in newly_enabled and not _covered(ts)]
|
||||
|
||||
@@ -198,9 +198,9 @@ def _print_tools_list(enabled_toolsets: set, mcp_servers: dict, platform: str =
|
||||
status = color("✓ enabled", Colors.GREEN) if ts_key in enabled_toolsets else color("✗ disabled", Colors.RED)
|
||||
print(f" {status} {ts_key} {color(label, Colors.DIM)}")
|
||||
if ts_key in (stripped or {}) and ts_key in enabled_toolsets:
|
||||
tools, by = stripped[ts_key]
|
||||
print(color(f" ⚠ {', '.join(tools)} removed by agent.disabled_toolsets: {', '.join(by)}"
|
||||
f" (run `hermes tools enable {ts_key}` to restore)", Colors.YELLOW))
|
||||
tools, by, hint = stripped[ts_key]
|
||||
print(color(f" ⚠ {', '.join(tools)} removed by agent.disabled_toolsets: {', '.join(by)} ({hint})",
|
||||
Colors.YELLOW))
|
||||
|
||||
print(f"Built-in toolsets ({platform}):")
|
||||
_print_rows((k, l) for k, l, _ in effective if k in builtin_keys)
|
||||
|
||||
@@ -18,11 +18,15 @@ def test_blank_slate_list_excludes_search_but_keeps_kanban():
|
||||
|
||||
def test_legacy_search_entry_is_reported_then_cleared_by_enabling_web():
|
||||
from model_tools import get_tool_definitions
|
||||
# Configs written by Blank Slate before this fix still carry `search` next to an enabled `web`.
|
||||
# Configs written by Blank Slate before this fix still carry `search` next to an enabled `web`; legacy
|
||||
# names (`terminal_tools`) strip tools the same way and must clear the same way.
|
||||
config = {"platform_toolsets": {"cli": ["file", "terminal", "web"]},
|
||||
"agent": {"disabled_toolsets": ["search", "browser", "memory"]}}
|
||||
"agent": {"disabled_toolsets": ["search", "terminal_tools", "browser", "memory"]}}
|
||||
enabled = _get_platform_tools(config, "cli", include_default_mcp_servers=False)
|
||||
assert toolsets_partially_stripped(config, enabled) == {"web": (["web_search"], ["search"])}
|
||||
assert toolsets_partially_stripped(config, enabled) == {
|
||||
"terminal": (["terminal"], ["terminal_tools"], "run `hermes tools enable terminal` to restore"),
|
||||
"web": (["web_search"], ["search"], "run `hermes tools enable web` to restore"),
|
||||
}
|
||||
|
||||
with patch("hermes_cli.tools_config.save_config"):
|
||||
_save_platform_tools(config, "cli", set(enabled))
|
||||
@@ -31,4 +35,4 @@ def test_legacy_search_entry_is_reported_then_cleared_by_enabling_web():
|
||||
assert toolsets_partially_stripped(config, enabled) == {}
|
||||
names = {t["function"]["name"] for t in get_tool_definitions(
|
||||
enabled_toolsets=sorted(enabled), disabled_toolsets=config["agent"]["disabled_toolsets"], quiet_mode=True)}
|
||||
assert {"web_search", "web_extract"} <= names
|
||||
assert {"web_search", "web_extract", "terminal"} <= names
|
||||
Reference in new issue
Block a user