diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 640179b0b4542..95cf19acadce3 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2490,8 +2490,18 @@ 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 = read_raw_config() + # Plugin toolsets (e.g. ``eikon``, ``buzz``, ``a2a``) register in the + # tool registry only after plugins load — which is after this validation + # runs — so ``validate_toolset`` alone flags them as unknown here. They + # are tracked in ``known_plugin_toolsets`` (the persisted per-platform + # plugin-toolset set written by the `hermes tools` save flow), so treat + # those names as valid too — but keyed per-platform, so a plugin known + # only for one platform cannot mask an invalid entry on another. See + # #81163, #38798. ts_warnings = validate_platform_toolsets( - read_raw_config().get("platform_toolsets"), validate_toolset + raw_cfg.get("platform_toolsets"), validate_toolset, + raw_cfg.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 4b72ec6be0dc7..95eb8155ac702 100644 --- a/hermes_cli/toolset_validation.py +++ b/hermes_cli/toolset_validation.py @@ -18,6 +18,7 @@ def validate_platform_toolsets( platform_toolsets: object, is_valid_toolset: Callable[[str], bool], + plugin_toolset_names: object = None, ) -> List[str]: """Return human-readable warnings for a ``platform_toolsets`` mapping. @@ -38,6 +39,20 @@ 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. + plugin_toolset_names: Optional iterable or per-platform mapping of + plugin-registered toolset names (e.g. ``eikon``, ``buzz``, ``a2a``). + These only appear in the tool registry after plugins load — which + is after config-time validation runs — so ``is_valid_toolset`` + alone flags them as unknown even though they resolve fine at + runtime. Two accepted shapes: + + - An iterable of names: every name is treated as valid for *every* + platform (legacy behavior; callers passing ``known_plugin_toolsets`` + as a flat union use this). + - A ``{platform: [names]}`` mapping: names are valid only for their + own platform. This preserves the per-platform precision that + ``known_plugin_toolsets`` encodes — a plugin known only for one + platform cannot mask an invalid entry on another. Returns: A list of warning strings (empty when everything is valid). @@ -46,13 +61,32 @@ def validate_platform_toolsets( if not isinstance(platform_toolsets, dict) or not platform_toolsets: return warnings + # Normalize plugin names to per-platform lookup. Mapping input is used + # keyed by platform; a flat iterable is replicated to every platform so + # both shapes share one code path below. + if isinstance(plugin_toolset_names, dict): + known_plugin = { + platform: {n for n in names if isinstance(n, str)} + for platform, names in plugin_toolset_names.items() + } + else: + _flat = { + n for n in (plugin_toolset_names or ()) if isinstance(n, str) + } + known_plugin = {platform: set(_flat) for platform in platform_toolsets} + + def _is_valid(name: str, platform: str) -> bool: + return bool(is_valid_toolset(name)) or name in known_plugin.get( + platform, () + ) + valid_count = 0 for platform, raw in platform_toolsets.items(): names = raw if isinstance(raw, list) else [raw] for name in names: if not isinstance(name, str) or not name: continue - if is_valid_toolset(name): + if _is_valid(name, platform): valid_count += 1 continue suggestion = f"hermes-{platform}" diff --git a/tests/hermes_cli/test_toolset_validation.py b/tests/hermes_cli/test_toolset_validation.py index 13e7f987170af..d5e715be4a514 100644 --- a/tests/hermes_cli/test_toolset_validation.py +++ b/tests/hermes_cli/test_toolset_validation.py @@ -46,5 +46,57 @@ def test_mixed_valid_and_invalid_flags_only_the_invalid(): assert "unknown toolset 'bogus'" in warnings[0] +def test_plugin_registered_toolsets_are_not_flagged_unknown(): + # Plugin toolsets (e.g. eikon, buzz, a2a) register in the tool registry + # only after plugins load — which is after config-time validation runs — + # so ``is_valid_toolset`` alone flags them as unknown even though they + # resolve fine at runtime. Names carried via ``plugin_toolset_names`` + # (the union of ``known_plugin_toolsets``) must validate clean. See #81163. + cfg = { + "cli": ["hermes-cli", "eikon", "buzz", "a2a"], + "discord": ["hermes-discord", "eikon"], + } + warnings = validate_platform_toolsets( + cfg, _is_valid, plugin_toolset_names=["a2a", "buzz", "eikon", "spotify"] + ) + assert warnings == [] + + +def test_plugin_toolset_names_do_not_mask_real_corruption(): + # A genuinely corrupted name is still flagged even when plugin toolsets + # are supplied — the plugin set only widens what is considered valid. + cfg = {"cli": ["hermes-cli", "eikon", "hermes"]} + warnings = validate_platform_toolsets( + cfg, _is_valid, plugin_toolset_names=["eikon", "buzz"] + ) + assert any("unknown toolset 'hermes'" in w for w in warnings) + + +def test_per_platform_plugin_names_do_not_cross_mask(): + # A plugin known only for one platform must NOT mask an invalid entry + # on another. Before the per-platform fix, the flat union carried ``a2a`` + # (known only for ``discord``) to every platform, silently accepting the + # genuinely wrong ``a2a`` entry on ``cli`` — the exact live-config case. + cfg = {"cli": ["hermes-cli", "a2a"], "discord": ["hermes-discord", "a2a"]} + known = {"cli": ["eikon", "buzz"], "discord": ["a2a", "eikon"]} + warnings = validate_platform_toolsets( + cfg, _is_valid, plugin_toolset_names=known + ) + # discord's a2a is legitimately known; cli's a2a is not. + assert not any("platform 'discord'" in w for w in warnings) + assert any("platform 'cli'" in w and "unknown toolset 'a2a'" in w for w in warnings) + + +def test_per_platform_mapping_still_accepts_known_plugin(): + # Same mapping shape as production: names valid under their own platform + # validate clean, matching the pre-existing flat-union behavior. + cfg = {"cli": ["hermes-cli", "eikon"], "discord": ["hermes-discord", "a2a"]} + known = {"cli": ["eikon", "buzz"], "discord": ["a2a", "eikon"]} + warnings = validate_platform_toolsets( + cfg, _is_valid, plugin_toolset_names=known + ) + assert warnings == [] + +