Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion hermes_cli/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
36 changes: 35 additions & 1 deletion hermes_cli/toolset_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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).
Expand All @@ -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}"
Expand Down
52 changes: 52 additions & 0 deletions tests/hermes_cli/test_toolset_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 == []