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
9 changes: 7 additions & 2 deletions hermes_cli/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
33 changes: 30 additions & 3 deletions hermes_cli/toolset_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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-<platform>`` 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
Expand All @@ -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:
Expand Down
43 changes: 43 additions & 0 deletions tests/hermes_cli/test_toolset_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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