diff --git a/hermes_cli/config.py b/hermes_cli/config.py index dcce55b51cda..1fdf13175999 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2297,6 +2297,7 @@ def _ensure_hermes_home_managed(home: Path): "allowed_channels": "", # If set, bot ONLY responds in these channel IDs (whitelist) "auto_thread": True, # Auto-create threads on @mention in channels (like Slack) "thread_require_mention": False, # If True, require @mention in threads too (multi-bot threads) + "bots_require_inline_mention": False, # Multi-bot rooms: if True, another bot must type @thisbot in its message to trigger a reply; a Discord reply/quote alone won't. Prevents two bots auto-replying to each other forever. Does not affect humans. "history_backfill": True, # If True, prepend recent channel scrollback when bot is triggered (recovers messages missed while require_mention gated them out) "history_backfill_limit": 50, # Max number of recent messages to scan when assembling the backfill block "reactions": True, # Add 👀/✅/❌ reactions to messages during processing @@ -5723,8 +5724,11 @@ def migrate_config(interactive: bool = True, quiet: bool = False) -> Dict[str, A from toolsets import validate_toolset from hermes_cli.toolset_validation import validate_platform_toolsets + _raw_cfg_for_validation = read_raw_config() ts_warnings = validate_platform_toolsets( - read_raw_config().get("platform_toolsets"), validate_toolset + _raw_cfg_for_validation.get("platform_toolsets"), + validate_toolset, + _raw_cfg_for_validation.get("known_plugin_toolsets"), ) for w in ts_warnings: results["warnings"].append(w) diff --git a/hermes_cli/toolset_validation.py b/hermes_cli/toolset_validation.py index cce814079123..9326ee08a754 100644 --- a/hermes_cli/toolset_validation.py +++ b/hermes_cli/toolset_validation.py @@ -10,23 +10,36 @@ or log entry — the agent degraded to text-only replies and the cause took significant debugging to find. Surfacing invalid toolset names (and the zero-tools end state) loudly turns that silent failure into an actionable one. + +Also cross-references ``known_plugin_toolsets`` (see +``hermes_cli/tools_config.py``): a name that ``is_valid_toolset`` rejects +today but was recorded there for the same platform is almost always a +disabled/uninstalled plugin, not a typo — the two cases warrant different +"did you mean" advice. """ -from typing import Callable, Dict, List +from typing import Callable, Dict, List, Optional def validate_platform_toolsets( platform_toolsets: object, is_valid_toolset: Callable[[str], bool], + known_plugin_toolsets: Optional[object] = None, ) -> List[str]: """Return human-readable warnings for a ``platform_toolsets`` mapping. Two 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 + renamed entry, or a plugin toolset whose plugin is currently disabled or + missing. When ``hermes-`` would have been valid (the exact #38798 shape, where ``cli`` held ``hermes`` instead of ``hermes-cli``), - the warning includes that as a suggestion. + the warning includes that as a suggestion. When the name instead matches + an entry previously recorded for this platform in + ``known_plugin_toolsets`` (populated by ``hermes tools`` — see + ``_save_platform_tools`` in ``hermes_cli/tools_config.py``), the warning + points at the plugin being disabled/uninstalled instead, since a stale + ``hermes-`` guess would be misleading there. 2. The mapping is non-empty but resolves to *zero* valid toolsets, so the agent would start with no tools at all. @@ -38,6 +51,11 @@ def validate_platform_toolsets( ``dict`` values carry toolset entries; anything else yields no warnings (nothing to validate). is_valid_toolset: Predicate returning ``True`` for a known toolset name. + known_plugin_toolsets: The raw ``known_plugin_toolsets`` value from + config — a ``{platform: [toolset_key, ...]}`` mapping of plugin + toolset keys seen the last time ``hermes tools`` saved that + platform. Optional; a missing/malformed value just skips the + cross-list check (falls back to the generic warning). Returns: A list of warning strings (empty when everything is valid). @@ -46,15 +64,30 @@ def validate_platform_toolsets( if not isinstance(platform_toolsets, dict) or not platform_toolsets: return warnings + if not isinstance(known_plugin_toolsets, dict): + known_plugin_toolsets = {} + valid_count = 0 for platform, raw in platform_toolsets.items(): names = raw if isinstance(raw, list) else [raw] + known_for_platform = known_plugin_toolsets.get(platform) + known_for_platform = ( + set(known_for_platform) if isinstance(known_for_platform, list) else set() + ) for name in names: if not isinstance(name, str) or not name: continue if is_valid_toolset(name): valid_count += 1 continue + if name in known_for_platform: + warnings.append( + f"platform '{platform}' references toolset '{name}', " + "which was previously provided by a plugin but is not " + "currently available — check that the plugin is still " + "enabled (plugins.enabled) and installed" + ) + continue suggestion = f"hermes-{platform}" hint = ( f" — did you mean '{suggestion}'?" diff --git a/tests/hermes_cli/test_toolset_validation.py b/tests/hermes_cli/test_toolset_validation.py index 7662b5b00d48..c51352195f6e 100644 --- a/tests/hermes_cli/test_toolset_validation.py +++ b/tests/hermes_cli/test_toolset_validation.py @@ -89,3 +89,51 @@ def test_real_validate_toolset_treats_hermes_cli_valid_and_hermes_invalid(): assert validate_toolset("hermes") is False warnings = validate_platform_toolsets({"cli": ["hermes"]}, validate_toolset) assert any("did you mean 'hermes-cli'?" in w for w in warnings) + + +# --- known_plugin_toolsets cross-list checks --------------------------------- +# +# A name that is_valid_toolset() rejects but that known_plugin_toolsets +# recorded for the same platform is (almost certainly) a plugin that used to +# provide that toolset and is now disabled or uninstalled — not a typo, so the +# `hermes-` guess is the wrong advice for it. + + +def test_disabled_plugin_toolset_gets_plugin_specific_warning(): + cfg = {"cli": ["hermes-cli", "weather-tools"]} + known = {"cli": ["weather-tools"]} + warnings = validate_platform_toolsets(cfg, _is_valid, known) + assert not any("zero valid toolsets" in w for w in warnings) + assert len(warnings) == 1 + assert "platform 'cli'" in warnings[0] + assert "toolset 'weather-tools'" in warnings[0] + assert "plugin" in warnings[0] + # Must not also carry (or be conflated with) the typo-guess wording. + assert "did you mean" not in warnings[0] + assert "unknown toolset" not in warnings[0] + + +def test_known_plugin_toolset_scoped_to_its_own_platform(): + # 'weather-tools' is only recorded as known for 'telegram', not 'cli' — a + # bogus 'cli' entry with the same name is still a plain unknown-toolset, + # not a disabled-plugin case, since it was never known there. + cfg = {"cli": ["weather-tools"]} + known = {"telegram": ["weather-tools"]} + warnings = validate_platform_toolsets(cfg, _is_valid, known) + unknown = [w for w in warnings if "unknown toolset 'weather-tools'" in w] + assert len(unknown) == 1 + + +def test_missing_known_plugin_toolsets_falls_back_to_generic_warning(): + # known_plugin_toolsets omitted entirely (None) — existing behavior for + # callers that don't pass it (e.g. before this field existed in config). + cfg = {"cli": ["bogus"]} + warnings = validate_platform_toolsets(cfg, _is_valid) + assert any("unknown toolset 'bogus'" in w for w in warnings) + + +@pytest.mark.parametrize("value", [None, [], "cli", 42, {"cli": "not-a-list"}]) +def test_malformed_known_plugin_toolsets_is_tolerated(value): + cfg = {"cli": ["bogus"]} + warnings = validate_platform_toolsets(cfg, _is_valid, value) + assert any("unknown toolset 'bogus'" in w for w in warnings)