diff --git a/model_tools.py b/model_tools.py index 0618138aa9a..de23bd6dc79 100644 --- a/model_tools.py +++ b/model_tools.py @@ -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: diff --git a/tests/test_model_tools.py b/tests/test_model_tools.py index 91e7103aac7..ddabfdbea89 100644 --- a/tests/test_model_tools.py +++ b/tests/test_model_tools.py @@ -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() diff --git a/toolsets.py b/toolsets.py index f33be147e95..5eef53af2d1 100644 --- a/toolsets.py +++ b/toolsets.py @@ -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.