diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 4669b49e8e059..c258f73995c09 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -1404,14 +1404,19 @@ def _disable_suspicious_mcp_servers(results: Dict[str, Any], quiet: bool) -> Non def _warn_invalid_platform_toolsets(results: Dict[str, Any], quiet: bool) -> None: """Surface invalid toolset names in platform_toolsets: ``resolve_toolset()`` returns [] for an - unknown name, silently disabling the affected tools. Best-effort; never blocks migration.""" + unknown name, silently disabling the affected tools. Best-effort; never blocks migration. + + ``known_plugin_toolsets`` rides along so plugin toolsets — which register only after plugins + load, i.e. after this runs — are not reported as unknown on every migration.""" 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 + raw_cfg = read_raw_config() for w in validate_platform_toolsets( - read_raw_config().get("platform_toolsets"), validate_toolset, toolset_allowed_for_platform): + raw_cfg.get("platform_toolsets"), validate_toolset, toolset_allowed_for_platform, + raw_cfg.get("known_plugin_toolsets")): results["warnings"].append(w) if not quiet: print(f" ⚠ {w}") diff --git a/hermes_cli/toolset_validation.py b/hermes_cli/toolset_validation.py index 72cbc3a6b9717..759d773da74b1 100644 --- a/hermes_cli/toolset_validation.py +++ b/hermes_cli/toolset_validation.py @@ -28,22 +28,46 @@ def _platform_default_is_valid( return False +def _known_plugin_toolsets(known: object, platform: object) -> set: + """Plugin toolset names *known_plugin_toolsets* declares for *platform* (empty when absent). + + ``hermes tools`` writes this section on every save, so it is the config's own record of which + plugin-provided toolsets are legitimate. Scoped per platform on purpose: a name declared for + one platform must not mask a typo under another. + """ + if not isinstance(known, dict): + return set() + names = known.get(platform) + if isinstance(names, str): # legacy single-string entry + return {names} + if not isinstance(names, list): + return set() + return {n for n in names if isinstance(n, str) and n} + + 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, + known_plugin_toolsets: object = None, ) -> List[str]: """Return human-readable warnings for a ``platform_toolsets`` mapping. Reports: a toolset name ``is_valid_toolset`` rejects (suggesting ``hermes-`` when that would have been valid); a non-empty mapping resolving to zero valid toolsets (agent would start with no tools); a platform with no valid toolsets, checked per-platform because the global net is suppressed once any platform is valid; and non-list platform values, which fall back to the platform - default. ``is_valid_toolset`` is injected so this does no registry imports or I/O.""" + default. ``is_valid_toolset`` is injected so this does no registry imports or I/O. + + ``known_plugin_toolsets`` is the raw config section of the same name (``{platform: [name, ...]}``). + Plugin toolsets register only after plugins load, which is after config validation runs, so + ``is_valid_toolset`` alone reports every one of them as unknown; names listed there for the + platform are accepted rather than warned about.""" warnings: List[str] = [] if not isinstance(platform_toolsets, dict) or not platform_toolsets: return warnings valid_count = 0 for platform, raw in platform_toolsets.items(): + known = _known_plugin_toolsets(known_plugin_toolsets, platform) default = _platform_default_toolset(platform) default_valid = _platform_default_is_valid(platform, default, is_valid_toolset, is_allowed_for_platform) platform_valid_count = 0 @@ -68,10 +92,13 @@ def validate_platform_toolsets( for name in raw: if not isinstance(name, str) or not name: continue - if not is_valid_toolset(name): + # A declared plugin toolset is valid even though the registry cannot confirm it yet, + # and is_allowed_for_platform would reject it for the same not-yet-loaded reason. + known_plugin = name in known + if not is_valid_toolset(name) and not known_plugin: hint = f" — did you mean '{default}'?" if default_valid else "" warnings.append(f"platform '{platform}' references unknown toolset '{name}'{hint}") - elif is_allowed_for_platform(name, str(platform)): + elif known_plugin or is_allowed_for_platform(name, str(platform)): valid_count += 1 platform_valid_count += 1 else: diff --git a/tests/hermes_cli/test_toolset_validation.py b/tests/hermes_cli/test_toolset_validation.py index 6663d1bcfb088..1a0987ce8a5ce 100644 --- a/tests/hermes_cli/test_toolset_validation.py +++ b/tests/hermes_cli/test_toolset_validation.py @@ -197,3 +197,46 @@ def test_populated_platforms_produce_no_empty_list_warning(): cfg = {"cli": ["hermes-cli"], "telegram": ["hermes-telegram"]} warnings = validate_platform_toolsets(cfg, _is_valid) assert warnings == [] + + +# --- known_plugin_toolsets: plugin toolsets register after config validation runs --- + +def test_declared_plugin_toolset_is_not_flagged_unknown(): + """A plugin toolset registers only after plugins load — long after this validation runs. + + `known_plugin_toolsets` is the config's own record (written by the `hermes tools` save flow) + that the name is legitimate, so it must not be reported as unknown. + """ + cfg = {"cli": ["a2a", "hermes-cli"]} + known = {"cli": ["a2a"]} + warnings = validate_platform_toolsets(cfg, _is_valid, known_plugin_toolsets=known) + assert not any("a2a" in w for w in warnings) + + +def test_declared_plugin_toolset_counts_as_a_valid_toolset(): + """It must also satisfy the zero-valid-toolsets net, or silencing one warning fires another.""" + warnings = validate_platform_toolsets({"cli": ["a2a"]}, _is_valid, known_plugin_toolsets={"cli": ["a2a"]}) + assert warnings == [] + + +def test_plugin_toolset_declared_for_another_platform_still_warns(): + """Per-platform scoping: one platform's plugin names must not mask another's typo.""" + cfg = {"cli": ["hermes-cli"], "discord": ["a2a"]} + warnings = validate_platform_toolsets(cfg, _is_valid, known_plugin_toolsets={"cli": ["a2a"]}) + assert any("unknown toolset 'a2a'" in w and "'discord'" in w for w in warnings) + + +def test_omitting_known_plugin_toolsets_preserves_existing_behavior(): + """Default None must behave exactly as before this parameter existed.""" + assert (validate_platform_toolsets({"cli": ["a2a"]}, _is_valid) + == validate_platform_toolsets({"cli": ["a2a"]}, _is_valid, known_plugin_toolsets=None)) + assert any("unknown toolset 'a2a'" in w for w in validate_platform_toolsets({"cli": ["a2a"]}, _is_valid)) + + +@pytest.mark.parametrize("known", [None, {}, {"cli": None}, {"cli": "a2a"}, {"cli": 42}, "nonsense", []]) +def test_malformed_known_plugin_toolsets_never_raises(known): + """Config is user-edited YAML: any shape must degrade to 'not declared', never crash migration.""" + warnings = validate_platform_toolsets({"cli": ["a2a"]}, _is_valid, known_plugin_toolsets=known) + # The legacy single-string shape is the one that legitimately suppresses the warning. + expected_suppressed = known == {"cli": "a2a"} + assert any("unknown toolset 'a2a'" in w for w in warnings) is not expected_suppressed