From a98191be3ede759e6748671a4209300de36e15fb Mon Sep 17 00:00:00 2001 From: chrisluersen <57424623+chrisluersen@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:52:49 -0400 Subject: [PATCH 1/2] fix(config): stop flagging plugin toolsets as unknown in platform_toolsets validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plugin-registered toolsets (eikon, buzz, a2a) only enter the tool registry after plugins load — which is after config-time validation runs in migrate_config(). validate_toolset() therefore flags them as unknown even though they resolve fine at runtime, spamming every startup with: platform 'cli' references unknown toolset 'a2a' — did you mean 'hermes-cli'? Treat names recorded in known_plugin_toolsets (the per-platform plugin-toolset set persisted by the `hermes tools` save flow) as valid too. The #38798 corruption check still fires for genuinely unknown names. --- hermes_cli/config.py | 19 ++++++++++++++- hermes_cli/toolset_validation.py | 15 +++++++++++- tests/hermes_cli/test_toolset_validation.py | 26 +++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 640179b0b4542..eb8dd2adbf3e7 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2490,8 +2490,25 @@ 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. See #81163. + _known_plugin = (raw_cfg.get("known_plugin_toolsets") or {}) or {} + _plugin_names: set = set() + for _names in _known_plugin.values(): + if isinstance(_names, str): + _plugin_names.add(_names) + elif isinstance(_names, list): + _plugin_names.update( + _n for _n in _names if isinstance(_n, str) + ) + ts_warnings = validate_platform_toolsets( - read_raw_config().get("platform_toolsets"), validate_toolset + raw_cfg.get("platform_toolsets"), validate_toolset, _plugin_names ) 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..000ef6df64c2c 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,13 @@ 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 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. Treat any name + carried here as valid too (callers pass the union of + ``known_plugin_toolsets`` from config). Returns: A list of warning strings (empty when everything is valid). @@ -46,13 +54,18 @@ def validate_platform_toolsets( if not isinstance(platform_toolsets, dict) or not platform_toolsets: return warnings + known_plugin = set(plugin_toolset_names) if plugin_toolset_names else set() + + def _is_valid(name: str) -> bool: + return bool(is_valid_toolset(name)) or name in known_plugin + 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): 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..31e3d989b6063 100644 --- a/tests/hermes_cli/test_toolset_validation.py +++ b/tests/hermes_cli/test_toolset_validation.py @@ -46,5 +46,31 @@ 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) + + From 05b2277c0bb44f220983c825d48637f1a202dee1 Mon Sep 17 00:00:00 2001 From: chrisluersen <57424623+chrisluersen@users.noreply.github.com> Date: Sat, 29 Aug 2026 03:29:30 -0400 Subject: [PATCH 2/2] fix(config): per-platform plugin toolset validation (no cross-platform masking) The prior fix collected the union of all platforms' known_plugin_toolsets and treated every name as valid for every platform, which silently masked a genuinely wrong per-platform entry (live config: 'a2a' on cli, only known for discord). validate_platform_toolsets now accepts a per-platform mapping and resolves plugin names against the current platform only, while keeping flat-iterable backward compat for existing callers. Adds regression tests: per-platform names do not cross-mask, and the mapping shape still accepts known plugins. --- hermes_cli/config.py | 17 +++----- hermes_cli/toolset_validation.py | 43 +++++++++++++++------ tests/hermes_cli/test_toolset_validation.py | 26 +++++++++++++ 3 files changed, 63 insertions(+), 23 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index eb8dd2adbf3e7..95cf19acadce3 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2496,19 +2496,12 @@ def migrate_config(interactive: bool = True, quiet: bool = False) -> Dict[str, A # 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. See #81163. - _known_plugin = (raw_cfg.get("known_plugin_toolsets") or {}) or {} - _plugin_names: set = set() - for _names in _known_plugin.values(): - if isinstance(_names, str): - _plugin_names.add(_names) - elif isinstance(_names, list): - _plugin_names.update( - _n for _n in _names if isinstance(_n, str) - ) - + # 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( - raw_cfg.get("platform_toolsets"), validate_toolset, _plugin_names + 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 000ef6df64c2c..95eb8155ac702 100644 --- a/hermes_cli/toolset_validation.py +++ b/hermes_cli/toolset_validation.py @@ -39,13 +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 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. Treat any name - carried here as valid too (callers pass the union of - ``known_plugin_toolsets`` from config). + 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). @@ -54,10 +61,24 @@ def validate_platform_toolsets( if not isinstance(platform_toolsets, dict) or not platform_toolsets: return warnings - known_plugin = set(plugin_toolset_names) if plugin_toolset_names else set() + # 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) -> bool: - return bool(is_valid_toolset(name)) or name in known_plugin + 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(): @@ -65,7 +86,7 @@ def _is_valid(name: str) -> bool: for name in names: if not isinstance(name, str) or not name: continue - if _is_valid(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 31e3d989b6063..d5e715be4a514 100644 --- a/tests/hermes_cli/test_toolset_validation.py +++ b/tests/hermes_cli/test_toolset_validation.py @@ -72,5 +72,31 @@ def test_plugin_toolset_names_do_not_mask_real_corruption(): 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 == [] + +