fix(tools): preserve core tools when a platform bundle is disabled

When a platform-bundle name (e.g. `hermes-yuanbao`, or any `hermes-*`) lands
in `agent.disabled_toolsets`, the shared tool-assembly path
(`model_tools._compute_tool_definitions`, used by the gateway, cron, AND the
CLI) subtracted the WHOLE bundle from the enabled set. Because every platform
bundle is defined as `_HERMES_CORE_TOOLS + [platform extras]`, and core tools
are shared by every other enabled toolset, the subtraction emptied the tool
list entirely — the model received `tools: []` / `tool_choice: null` and
started replying "I cannot execute shell commands" with no error, no warning,
and `hermes tools list` / `hermes doctor` still green. For unattended cron
jobs this fails silently for days. (#33924)

(The original report framed this as gateway-only; it actually affects every
caller of `_compute_tool_definitions`, including the CLI — the reporter's
follow-up confirms this. Fixing the shared chokepoint covers all paths.)

Fix: for a `hermes-*` bundle in `disabled_toolsets`, subtract only its
*non-core delta* (its platform-specific tools plus those of any `includes`),
leaving `_HERMES_CORE_TOOLS` intact. Disabling a bundle now removes its
platform tools (e.g. the `yb_*` tools for `hermes-yuanbao`) while terminal,
read_file, web, etc. survive. A `logger.warning` notes that core tools are
preserved and that bundle names usually belong in `toolsets:`, not
`disabled_toolsets` — informative, not destructive (the subtraction still
behaves sensibly).

Salvaged from #33941 by @liuhao1024 (authorship preserved). Extracted the
inline bundle-resolution into a module-level `_bundle_non_core_tools` helper
(was re-importing `toolsets` inside the disable loop), and added the
informative warning folding in the UX intent of #34073 (@ousiaresearch)
without its hard "ignore the bundle name" behavior — which would have undone
this fix's sensible-subtraction.

Verified empirically: disabling `hermes-yuanbao` from a gateway-style enabled
set keeps all core tools (18→18) and would remove only the 5 `yb_*` tools;
disabling `hermes-discord` removes only `discord`/`discord_admin`.

Fixes #33924

Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
This commit is contained in:
liuhao1024
2026-06-21 16:22:55 +05:30
parent 7b9a0b315b
commit dd042fc4df
3 changed files with 134 additions and 2 deletions

View File

@@ -34,6 +34,10 @@ from toolsets import resolve_toolset, validate_toolset
logger = logging.getLogger(__name__)
# Tracks platform-bundle names already flagged in disabled_toolsets so the
# advisory (#33924) is logged once per name, not on every tool recompute.
_WARNED_DISABLED_BUNDLES: set = set()
# =============================================================================
# Async Bridging (single source of truth -- used by registry.dispatch too)
@@ -392,8 +396,29 @@ def _compute_tool_definitions(
if disabled_toolsets:
for toolset_name in disabled_toolsets:
if validate_toolset(toolset_name):
resolved = resolve_toolset(toolset_name)
tools_to_include.difference_update(resolved)
if toolset_name.startswith("hermes-"):
# Platform bundles (hermes-*) include _HERMES_CORE_TOOLS, so
# subtracting the whole bundle would strip core tools shared
# by other enabled toolsets and empty the tool list (#33924).
# Subtract only the bundle's non-core delta; keep core.
from toolsets import bundle_non_core_tools
to_remove = bundle_non_core_tools(toolset_name)
tools_to_include.difference_update(to_remove)
resolved = sorted(to_remove)
if not quiet_mode and toolset_name not in _WARNED_DISABLED_BUNDLES:
_WARNED_DISABLED_BUNDLES.add(toolset_name)
logger.info(
"agent.disabled_toolsets contains platform-bundle "
"name '%s'; core tools are preserved and only its "
"platform-specific tools (%s) are removed. Bundle "
"names usually belong in `toolsets:`, not "
"`disabled_toolsets` (#33924).",
toolset_name,
", ".join(resolved) if resolved else "none",
)
else:
resolved = resolve_toolset(toolset_name)
tools_to_include.difference_update(resolved)
if not quiet_mode:
print(f"🚫 Disabled toolset '{toolset_name}': {', '.join(resolved) if resolved else 'no tools'}")
elif toolset_name in _LEGACY_TOOLSET_MAP:

View File

@@ -457,3 +457,82 @@ class TestCoerceNumberInfNan:
assert _coerce_number("42") == 42
assert _coerce_number("3.14") == 3.14
assert _coerce_number("1e3") == 1000
class TestDisabledToolsetsPlatformBundle:
"""Regression test for #33924: disabling a platform bundle (hermes-*)
must not remove core tools from other enabled toolsets."""
def test_disabling_platform_bundle_preserves_core_tools(self):
"""Disabling hermes-yuanbao should not strip core tools from hermes-telegram."""
from model_tools import get_tool_definitions
tools_telegram = get_tool_definitions(
enabled_toolsets=["hermes-telegram"],
quiet_mode=True,
)
tools_telegram_no_yuanbao = get_tool_definitions(
enabled_toolsets=["hermes-telegram"],
disabled_toolsets=["hermes-yuanbao"],
quiet_mode=True,
)
names_telegram = {t["function"]["name"] for t in tools_telegram}
names_no_yuanbao = {t["function"]["name"] for t in tools_telegram_no_yuanbao}
# Disabling a *different* platform bundle must not remove any tools
assert names_telegram == names_no_yuanbao, (
f"Tools lost after disabling hermes-yuanbao: "
f"{names_telegram - names_no_yuanbao}"
)
def test_disabling_platform_bundle_removes_own_tools(self):
"""Disabling hermes-discord should remove discord-specific tools."""
from model_tools import get_tool_definitions
tools = get_tool_definitions(
enabled_toolsets=["hermes-discord"],
disabled_toolsets=["hermes-discord"],
quiet_mode=True,
)
names = {t["function"]["name"] for t in tools}
assert "discord" not in names
def test_disabling_non_platform_toolset_still_works(self):
"""Disabling a regular (non-hermes-) toolset still subtracts all tools."""
from model_tools import get_tool_definitions
tools_normal = get_tool_definitions(
enabled_toolsets=["hermes-telegram"],
quiet_mode=True,
)
tools_no_web = get_tool_definitions(
enabled_toolsets=["hermes-telegram"],
disabled_toolsets=["web"],
quiet_mode=True,
)
names_normal = {t["function"]["name"] for t in tools_normal}
names_no_web = {t["function"]["name"] for t in tools_no_web}
web_tools = {"web_search", "web_extract"}
removed = names_normal - names_no_web
# web tools should be removed (if they were present)
present_web = web_tools & names_normal
assert present_web <= removed, (
f"Web tools not removed: {present_web - removed}"
)
def test_disabling_bundle_removes_platform_tools_but_keeps_core(self):
"""Disabling hermes-discord (when enabled) removes discord/discord_admin
from the resolved delta but keeps core tools — via bundle_non_core_tools."""
from toolsets import bundle_non_core_tools, _HERMES_CORE_TOOLS
delta = bundle_non_core_tools("hermes-yuanbao")
# The delta is the bundle's platform-specific tools, NOT core.
assert "yb_send_dm" in delta
assert not (delta & set(_HERMES_CORE_TOOLS)), "core tools must not be in the removal delta"
def test_bundle_non_core_tools_unknown_falls_back(self):
"""An unknown/garbage bundle name falls back to full resolution (best effort)."""
from toolsets import bundle_non_core_tools
# A non-existent bundle resolves to an empty set (no tools), not a crash.
assert bundle_non_core_tools("hermes-does-not-exist") == set()

View File

@@ -627,6 +627,34 @@ def get_toolset(name: str) -> Optional[Dict[str, Any]]:
}
def bundle_non_core_tools(toolset_name: str) -> Set[str]:
"""Return a ``hermes-*`` bundle's platform-specific tools, excluding core.
Platform bundles are defined as ``_HERMES_CORE_TOOLS + [platform extras]``.
When a bundle name appears in ``disabled_toolsets``, subtracting the whole
bundle would strip core tools (terminal, read_file, …) shared by every
other enabled toolset, emptying the model's tool list (#33924). This
returns only the bundle's non-core delta (its own extras plus those of any
one-level ``includes``), so disabling a bundle removes its platform tools
while leaving core intact.
Bundle nesting is one level deep in practice (only ``hermes-gateway``
includes other bundles, and those leaves don't nest further), so a single
``includes`` pass is sufficient. Unknown/garbage names fall back to the
full resolution minus core — never re-introducing the core wipe.
"""
core = set(_HERMES_CORE_TOOLS)
ts_def = get_toolset(toolset_name)
if not (ts_def and "tools" in ts_def):
return set(resolve_toolset(toolset_name)) - core
to_remove = set(ts_def["tools"]) - core
for inc in ts_def.get("includes", []):
inc_def = get_toolset(inc)
if inc_def and "tools" in inc_def:
to_remove.update(set(inc_def["tools"]) - core)
return to_remove
def resolve_toolset(name: str, visited: Set[str] = None) -> List[str]:
"""
Recursively resolve a toolset to get all tool names.