diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 4d8863267f..5666f53c34 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2498,9 +2498,12 @@ def migrate_config(interactive: bool = True, quiet: bool = False) -> Dict[str, A try: from toolsets import validate_toolset from hermes_cli.toolset_validation import validate_platform_toolsets + from hermes_cli.toolset_scope import toolset_allowed_for_platform ts_warnings = validate_platform_toolsets( - read_raw_config().get("platform_toolsets"), validate_toolset + read_raw_config().get("platform_toolsets"), + validate_toolset, + toolset_allowed_for_platform, ) for w in ts_warnings: results["warnings"].append(w) diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index edbcf33359..10d433db7c 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -31,6 +31,10 @@ from hermes_cli.nous_subscription import ( get_nous_subscription_features, ) from hermes_cli.nous_account import format_nous_portal_entitlement_message +from hermes_cli.toolset_scope import ( + _TOOLSET_PLATFORM_RESTRICTIONS, + toolset_allowed_for_platform as _toolset_allowed_for_platform, +) from tools.tool_backend_helpers import NOUS_MANAGED_PROVIDER, fal_key_is_configured from utils import base_url_hostname, is_truthy_value @@ -205,28 +209,6 @@ def _homeassistant_credentials_present() -> bool: except Exception: return False -# Platform-scoped toolsets: only appear in the `hermes tools` checklist for -# these platforms, and only resolve/save for these platforms. A toolset -# absent from this map is available on every platform (current behaviour). -# -# Use this for tools whose APIs only make sense on one platform (Discord -# server admin, Slack workspace admin, etc.). Keeps every other platform's -# checklist from filling up with irrelevant toggles. -_TOOLSET_PLATFORM_RESTRICTIONS: Dict[str, Set[str]] = { - "discord": {"discord"}, - "discord_admin": {"discord"}, -} - - -def _toolset_allowed_for_platform(ts_key: str, platform: str) -> bool: - """Return True if ``ts_key`` is configurable on ``platform``. - - Toolsets without a restriction entry are allowed everywhere (the default). - """ - allowed = _TOOLSET_PLATFORM_RESTRICTIONS.get(ts_key) - return allowed is None or platform in allowed - - def _toolset_configuration_platform(ts_key: str, default: str = "cli") -> str: """Return the platform a platform-less configuration UI should target. diff --git a/hermes_cli/toolset_scope.py b/hermes_cli/toolset_scope.py new file mode 100644 index 0000000000..1085f3fcd4 --- /dev/null +++ b/hermes_cli/toolset_scope.py @@ -0,0 +1,20 @@ +"""Platform scope rules for configured toolsets. + +This module is intentionally independent of tool resolution and CLI setup so +configuration validation and runtime resolution apply the same platform policy. +""" + +from typing import Set + + +# Toolsets without a restriction entry are available on every platform. +_TOOLSET_PLATFORM_RESTRICTIONS = { + "discord": {"discord"}, + "discord_admin": {"discord"}, +} + + +def toolset_allowed_for_platform(ts_key: str, platform: str) -> bool: + """Return whether ``ts_key`` is available on ``platform``.""" + allowed: Set[str] | None = _TOOLSET_PLATFORM_RESTRICTIONS.get(ts_key) + return allowed is None or platform in allowed diff --git a/hermes_cli/toolset_validation.py b/hermes_cli/toolset_validation.py index 990cfe072d..e7a2b7cedd 100644 --- a/hermes_cli/toolset_validation.py +++ b/hermes_cli/toolset_validation.py @@ -14,14 +14,44 @@ zero-tools end state) loudly turns that silent failure into an actionable one. from typing import Callable, List +from hermes_cli.platforms import PLATFORMS +from hermes_cli.toolset_scope import toolset_allowed_for_platform + + +def _platform_default_toolset(platform: object) -> str: + info = PLATFORMS.get(platform) + return info.default_toolset if info is not None else f"hermes-{platform}" + + +def _platform_default_is_valid( + platform: object, + default_toolset: str, + is_valid_toolset: Callable[[str], bool], + is_allowed_for_platform: Callable[[str, str], bool], +) -> bool: + platform_name = str(platform) + if is_valid_toolset(default_toolset) and is_allowed_for_platform( + default_toolset, platform_name + ): + return True + # Dynamic plugin platforms are resolved by toolsets.resolve_toolset() even + # though their synthesized hermes- name is not in TOOLSETS. + try: + from gateway.platform_registry import platform_registry + + return platform_registry.is_registered(platform) + except Exception: + return False + def validate_platform_toolsets( platform_toolsets: object, is_valid_toolset: Callable[[str], bool], + is_allowed_for_platform: Callable[[str, str], bool] = toolset_allowed_for_platform, ) -> List[str]: """Return human-readable warnings for a ``platform_toolsets`` mapping. - Two failure modes are reported: + The following failure modes are reported: 1. A toolset name that ``is_valid_toolset`` rejects — usually a corrupted or renamed entry. When ``hermes-`` would have been valid (the exact @@ -29,12 +59,17 @@ def validate_platform_toolsets( the warning includes that as a suggestion. 2. The mapping is non-empty but resolves to *zero* valid toolsets, so the agent would start with no tools at all. - 3. A platform is configured with an *empty* toolset list. Checked - per-platform because the global zero-valid-toolsets net in (2) is - suppressed as soon as any other platform carries a valid toolset. + 3. A platform is configured with no valid toolsets. Checked per-platform + because the global zero-valid-toolsets net in (2) is suppressed as soon + as any other platform carries a valid toolset. This includes empty + lists and lists containing only invalid entries. Null values are also + reported, but match the resolver's platform-default fallback. + 4. A non-list platform value is malformed but matches the resolver's + platform-default fallback rather than selecting a toolset by itself. ``is_valid_toolset`` is injected (normally :func:`toolsets.validate_toolset`) - so this function performs no imports or I/O and is testable in isolation. + so this function performs no tool-registry imports or I/O and is testable in + isolation. Platform default metadata comes from the shared platform registry. Args: platform_toolsets: The raw ``platform_toolsets`` value from config. Only @@ -51,32 +86,59 @@ def validate_platform_toolsets( valid_count = 0 for platform, raw in platform_toolsets.items(): - # An explicitly-empty list is honoured verbatim by - # ``resolve_enabled_toolsets()``: ``[]`` *is* a list, so the - # platform-default fallback is skipped and the platform starts with zero - # tools. That is the intended contract for a deliberate opt-out, but it - # reads identically to an accidental wipe, and the global ``valid_count`` - # below cannot catch it — any *other* populated platform pushes the count - # above zero and suppresses the safety net. Report it per-platform so the - # zero-tools end state is never silent. - if isinstance(raw, list) and not raw: + platform_valid_count = 0 + if not isinstance(raw, list): + fallback = _platform_default_toolset(platform) + if _platform_default_is_valid( + platform, fallback, is_valid_toolset, is_allowed_for_platform + ): + valid_count += 1 + platform_valid_count += 1 + fallback_detail = f"falling back to '{fallback}'" + else: + fallback_detail = f"falling back to unknown default '{fallback}'" + if raw is None: + value_detail = "a null toolset value" + elif isinstance(raw, str): + value_detail = f"invalid toolset value '{raw}'" + else: + value_detail = f"invalid {type(raw).__name__} toolset value" warnings.append( - f"platform '{platform}' is configured with an empty toolset " - f"list — the agent will have no tools on this platform. " - f"Run `hermes tools` to reconfigure." + f"platform '{platform}' has {value_detail} — " + f"{fallback_detail}. Run `hermes tools` to configure explicitly." ) + if platform_valid_count == 0: + warnings.append( + f"platform '{platform}' has no valid toolsets configured — " + f"the agent will have no tools on this platform. " + f"Run `hermes tools` to reconfigure." + ) continue - names = raw if isinstance(raw, list) else [raw] + names = raw for name in names: if not isinstance(name, str) or not name: continue - if is_valid_toolset(name): + if is_valid_toolset(name) and is_allowed_for_platform( + name, str(platform) + ): valid_count += 1 + platform_valid_count += 1 continue - suggestion = f"hermes-{platform}" + if is_valid_toolset(name): + warnings.append( + f"platform '{platform}' references toolset '{name}' " + "which is not available on this platform" + ) + continue + suggestion = _platform_default_toolset(platform) hint = ( f" — did you mean '{suggestion}'?" - if is_valid_toolset(suggestion) + if _platform_default_is_valid( + platform, + suggestion, + is_valid_toolset, + is_allowed_for_platform, + ) else "" ) warnings.append( @@ -84,6 +146,16 @@ def validate_platform_toolsets( f"'{name}'{hint}" ) + if platform_valid_count == 0: + if isinstance(raw, list) and not raw: + reason = "is configured with an empty toolset list" + else: + reason = "has no valid toolsets configured" + warnings.append( + f"platform '{platform}' {reason} — the agent will have no " + f"tools on this platform. Run `hermes tools` to reconfigure." + ) + if valid_count == 0: warnings.append( "platform_toolsets resolves to zero valid toolsets — the agent will " diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index 6acef7d27e..8523bf3f94 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -74,6 +74,30 @@ def test_partially_valid_platform_toolsets_no_runtime_warning(caplog): assert not any("#38798" in r.getMessage() for r in caplog.records) +def test_null_platform_toolsets_fall_back_to_platform_default(): + """A YAML ``platform:`` value is absent, not an explicit empty list.""" + config = {"platform_toolsets": {"cli": None}} + + enabled = _get_platform_tools(config, "cli", include_default_mcp_servers=False) + default_enabled = _get_platform_tools( + {}, "cli", include_default_mcp_servers=False + ) + + assert enabled == default_enabled + + +def test_scalar_platform_toolsets_fall_back_to_platform_default(): + """A non-list platform value is ignored by the resolver.""" + config = {"platform_toolsets": {"cli": "bogus"}} + + enabled = _get_platform_tools(config, "cli", include_default_mcp_servers=False) + default_enabled = _get_platform_tools( + {}, "cli", include_default_mcp_servers=False + ) + + assert enabled == default_enabled + + diff --git a/tests/hermes_cli/test_toolset_validation.py b/tests/hermes_cli/test_toolset_validation.py index 4a1c470074..6663d1bcfb 100644 --- a/tests/hermes_cli/test_toolset_validation.py +++ b/tests/hermes_cli/test_toolset_validation.py @@ -14,6 +14,8 @@ _KNOWN = { "hermes-cli", "hermes-telegram", "hermes-discord", + "hermes-whatsapp", + "discord", "terminal", "web", } @@ -41,9 +43,14 @@ def test_mixed_valid_and_invalid_flags_only_the_invalid(): warnings = validate_platform_toolsets(cfg, _is_valid) # One valid entry exists, so no zero-valid warning. assert not any("zero valid toolsets" in w for w in warnings) - assert len(warnings) == 1 - assert "platform 'discord'" in warnings[0] - assert "unknown toolset 'bogus'" in warnings[0] + assert any( + "platform 'discord'" in w and "unknown toolset 'bogus'" in w + for w in warnings + ) + assert any( + "platform 'discord'" in w and "no valid toolsets" in w + for w in warnings + ) @@ -94,6 +101,98 @@ def test_empty_list_does_not_mask_unknown_names_on_other_platforms(): assert any("zero valid toolsets" in w for w in warnings) +def test_null_platform_warns_even_when_others_are_valid(): + # YAML's ``cli:`` parses as None. The resolver treats it as absent and uses + # the platform default, so the warning must not claim that tools disappear. + cfg = {"cli": None, "telegram": ["hermes-telegram"]} + warnings = validate_platform_toolsets(cfg, _is_valid) + + assert any( + "platform 'cli'" in w + and "null toolset value" in w + and "falling back to 'hermes-cli'" in w + for w in warnings + ) + assert not any("zero valid toolsets" in w for w in warnings) + + +def test_null_platform_uses_canonical_default_for_alias(): + cfg = {"whatsapp_cloud": None, "telegram": ["hermes-telegram"]} + warnings = validate_platform_toolsets(cfg, _is_valid) + + assert any( + "platform 'whatsapp_cloud'" in w + and "falling back to 'hermes-whatsapp'" in w + for w in warnings + ) + assert not any("zero valid toolsets" in w for w in warnings) + + +def test_scalar_platform_value_warns_but_uses_platform_default(): + cfg = {"cli": "bogus", "telegram": ["hermes-telegram"]} + warnings = validate_platform_toolsets(cfg, _is_valid) + + assert any( + "platform 'cli'" in w + and "invalid toolset value 'bogus'" in w + and "falling back to 'hermes-cli'" in w + for w in warnings + ) + assert not any("zero valid toolsets" in w for w in warnings) + + +def test_platform_restricted_toolset_warns_when_other_platform_is_valid(): + cfg = {"telegram": ["discord"], "cli": ["hermes-cli"]} + warnings = validate_platform_toolsets(cfg, _is_valid) + + assert any( + "platform 'telegram'" in w + and "toolset 'discord'" in w + and "not available" in w + for w in warnings + ) + assert not any("zero valid toolsets" in w for w in warnings) + + +def test_null_plugin_platform_uses_synthetic_default(): + from gateway.platform_registry import PlatformEntry, platform_registry + from toolsets import resolve_toolset + + platform = "toolset_validation_plugin" + platform_registry.register( + PlatformEntry( + name=platform, + label="Toolset Validation Plugin", + adapter_factory=lambda _config: object(), + check_fn=lambda: True, + ) + ) + try: + cfg = {platform: None, "telegram": ["hermes-telegram"]} + warnings = validate_platform_toolsets(cfg, _is_valid) + + assert resolve_toolset(f"hermes-{platform}") + assert any( + f"platform '{platform}'" in w + and f"falling back to 'hermes-{platform}'" in w + for w in warnings + ) + assert not any("zero valid toolsets" in w for w in warnings) + finally: + platform_registry.unregister(platform) + + +def test_all_invalid_platform_warns_even_when_others_are_valid(): + cfg = {"cli": ["bogus"], "telegram": ["hermes-telegram"]} + warnings = validate_platform_toolsets(cfg, _is_valid) + + assert any("unknown toolset 'bogus'" in w for w in warnings) + assert any( + "platform 'cli'" in w and "no valid toolsets" in w for w in warnings + ) + assert not any("zero valid toolsets" in w for w in warnings) + + def test_populated_platforms_produce_no_empty_list_warning(): cfg = {"cli": ["hermes-cli"], "telegram": ["hermes-telegram"]} warnings = validate_platform_toolsets(cfg, _is_valid)