fix(config): warn for empty platform toolsets
This commit is contained in:
@@ -2498,9 +2498,12 @@ def migrate_config(interactive: bool = True, quiet: bool = False) -> Dict[str, A
|
||||
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
|
||||
|
||||
ts_warnings = validate_platform_toolsets(
|
||||
read_raw_config().get("platform_toolsets"), validate_toolset
|
||||
read_raw_config().get("platform_toolsets"),
|
||||
validate_toolset,
|
||||
toolset_allowed_for_platform,
|
||||
)
|
||||
for w in ts_warnings:
|
||||
results["warnings"].append(w)
|
||||
|
||||
@@ -31,6 +31,10 @@ from hermes_cli.nous_subscription import (
|
||||
get_nous_subscription_features,
|
||||
)
|
||||
from hermes_cli.nous_account import format_nous_portal_entitlement_message
|
||||
from hermes_cli.toolset_scope import (
|
||||
_TOOLSET_PLATFORM_RESTRICTIONS,
|
||||
toolset_allowed_for_platform as _toolset_allowed_for_platform,
|
||||
)
|
||||
from tools.tool_backend_helpers import NOUS_MANAGED_PROVIDER, fal_key_is_configured
|
||||
from utils import base_url_hostname, is_truthy_value
|
||||
|
||||
@@ -205,28 +209,6 @@ def _homeassistant_credentials_present() -> bool:
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
# Platform-scoped toolsets: only appear in the `hermes tools` checklist for
|
||||
# these platforms, and only resolve/save for these platforms. A toolset
|
||||
# absent from this map is available on every platform (current behaviour).
|
||||
#
|
||||
# Use this for tools whose APIs only make sense on one platform (Discord
|
||||
# server admin, Slack workspace admin, etc.). Keeps every other platform's
|
||||
# checklist from filling up with irrelevant toggles.
|
||||
_TOOLSET_PLATFORM_RESTRICTIONS: Dict[str, Set[str]] = {
|
||||
"discord": {"discord"},
|
||||
"discord_admin": {"discord"},
|
||||
}
|
||||
|
||||
|
||||
def _toolset_allowed_for_platform(ts_key: str, platform: str) -> bool:
|
||||
"""Return True if ``ts_key`` is configurable on ``platform``.
|
||||
|
||||
Toolsets without a restriction entry are allowed everywhere (the default).
|
||||
"""
|
||||
allowed = _TOOLSET_PLATFORM_RESTRICTIONS.get(ts_key)
|
||||
return allowed is None or platform in allowed
|
||||
|
||||
|
||||
def _toolset_configuration_platform(ts_key: str, default: str = "cli") -> str:
|
||||
"""Return the platform a platform-less configuration UI should target.
|
||||
|
||||
|
||||
@@ -0,0 +1,20 @@
|
||||
"""Platform scope rules for configured toolsets.
|
||||
|
||||
This module is intentionally independent of tool resolution and CLI setup so
|
||||
configuration validation and runtime resolution apply the same platform policy.
|
||||
"""
|
||||
|
||||
from typing import Set
|
||||
|
||||
|
||||
# Toolsets without a restriction entry are available on every platform.
|
||||
_TOOLSET_PLATFORM_RESTRICTIONS = {
|
||||
"discord": {"discord"},
|
||||
"discord_admin": {"discord"},
|
||||
}
|
||||
|
||||
|
||||
def toolset_allowed_for_platform(ts_key: str, platform: str) -> bool:
|
||||
"""Return whether ``ts_key`` is available on ``platform``."""
|
||||
allowed: Set[str] | None = _TOOLSET_PLATFORM_RESTRICTIONS.get(ts_key)
|
||||
return allowed is None or platform in allowed
|
||||
@@ -14,14 +14,44 @@ zero-tools end state) loudly turns that silent failure into an actionable one.
|
||||
|
||||
from typing import Callable, List
|
||||
|
||||
from hermes_cli.platforms import PLATFORMS
|
||||
from hermes_cli.toolset_scope import toolset_allowed_for_platform
|
||||
|
||||
|
||||
def _platform_default_toolset(platform: object) -> str:
|
||||
info = PLATFORMS.get(platform)
|
||||
return info.default_toolset if info is not None else f"hermes-{platform}"
|
||||
|
||||
|
||||
def _platform_default_is_valid(
|
||||
platform: object,
|
||||
default_toolset: str,
|
||||
is_valid_toolset: Callable[[str], bool],
|
||||
is_allowed_for_platform: Callable[[str, str], bool],
|
||||
) -> bool:
|
||||
platform_name = str(platform)
|
||||
if is_valid_toolset(default_toolset) and is_allowed_for_platform(
|
||||
default_toolset, platform_name
|
||||
):
|
||||
return True
|
||||
# Dynamic plugin platforms are resolved by toolsets.resolve_toolset() even
|
||||
# though their synthesized hermes-<platform> name is not in TOOLSETS.
|
||||
try:
|
||||
from gateway.platform_registry import platform_registry
|
||||
|
||||
return platform_registry.is_registered(platform)
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
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,
|
||||
) -> List[str]:
|
||||
"""Return human-readable warnings for a ``platform_toolsets`` mapping.
|
||||
|
||||
Two failure modes are reported:
|
||||
The following failure modes are reported:
|
||||
|
||||
1. A toolset name that ``is_valid_toolset`` rejects — usually a corrupted or
|
||||
renamed entry. When ``hermes-<platform>`` would have been valid (the exact
|
||||
@@ -29,12 +59,17 @@ def validate_platform_toolsets(
|
||||
the warning includes that as a suggestion.
|
||||
2. The mapping is non-empty but resolves to *zero* valid toolsets, so the
|
||||
agent would start with no tools at all.
|
||||
3. A platform is configured with an *empty* toolset list. Checked
|
||||
per-platform because the global zero-valid-toolsets net in (2) is
|
||||
suppressed as soon as any other platform carries a valid toolset.
|
||||
3. A platform is configured with no valid toolsets. Checked per-platform
|
||||
because the global zero-valid-toolsets net in (2) is suppressed as soon
|
||||
as any other platform carries a valid toolset. This includes empty
|
||||
lists and lists containing only invalid entries. Null values are also
|
||||
reported, but match the resolver's platform-default fallback.
|
||||
4. A non-list platform value is malformed but matches the resolver's
|
||||
platform-default fallback rather than selecting a toolset by itself.
|
||||
|
||||
``is_valid_toolset`` is injected (normally :func:`toolsets.validate_toolset`)
|
||||
so this function performs no imports or I/O and is testable in isolation.
|
||||
so this function performs no tool-registry imports or I/O and is testable in
|
||||
isolation. Platform default metadata comes from the shared platform registry.
|
||||
|
||||
Args:
|
||||
platform_toolsets: The raw ``platform_toolsets`` value from config. Only
|
||||
@@ -51,32 +86,59 @@ def validate_platform_toolsets(
|
||||
|
||||
valid_count = 0
|
||||
for platform, raw in platform_toolsets.items():
|
||||
# An explicitly-empty list is honoured verbatim by
|
||||
# ``resolve_enabled_toolsets()``: ``[]`` *is* a list, so the
|
||||
# platform-default fallback is skipped and the platform starts with zero
|
||||
# tools. That is the intended contract for a deliberate opt-out, but it
|
||||
# reads identically to an accidental wipe, and the global ``valid_count``
|
||||
# below cannot catch it — any *other* populated platform pushes the count
|
||||
# above zero and suppresses the safety net. Report it per-platform so the
|
||||
# zero-tools end state is never silent.
|
||||
if isinstance(raw, list) and not raw:
|
||||
platform_valid_count = 0
|
||||
if not isinstance(raw, list):
|
||||
fallback = _platform_default_toolset(platform)
|
||||
if _platform_default_is_valid(
|
||||
platform, fallback, is_valid_toolset, is_allowed_for_platform
|
||||
):
|
||||
valid_count += 1
|
||||
platform_valid_count += 1
|
||||
fallback_detail = f"falling back to '{fallback}'"
|
||||
else:
|
||||
fallback_detail = f"falling back to unknown default '{fallback}'"
|
||||
if raw is None:
|
||||
value_detail = "a null toolset value"
|
||||
elif isinstance(raw, str):
|
||||
value_detail = f"invalid toolset value '{raw}'"
|
||||
else:
|
||||
value_detail = f"invalid {type(raw).__name__} toolset value"
|
||||
warnings.append(
|
||||
f"platform '{platform}' is configured with an empty toolset "
|
||||
f"list — the agent will have no tools on this platform. "
|
||||
f"Run `hermes tools` to reconfigure."
|
||||
f"platform '{platform}' has {value_detail} — "
|
||||
f"{fallback_detail}. Run `hermes tools` to configure explicitly."
|
||||
)
|
||||
if platform_valid_count == 0:
|
||||
warnings.append(
|
||||
f"platform '{platform}' has no valid toolsets configured — "
|
||||
f"the agent will have no tools on this platform. "
|
||||
f"Run `hermes tools` to reconfigure."
|
||||
)
|
||||
continue
|
||||
names = raw if isinstance(raw, list) else [raw]
|
||||
names = raw
|
||||
for name in names:
|
||||
if not isinstance(name, str) or not name:
|
||||
continue
|
||||
if is_valid_toolset(name):
|
||||
if is_valid_toolset(name) and is_allowed_for_platform(
|
||||
name, str(platform)
|
||||
):
|
||||
valid_count += 1
|
||||
platform_valid_count += 1
|
||||
continue
|
||||
suggestion = f"hermes-{platform}"
|
||||
if is_valid_toolset(name):
|
||||
warnings.append(
|
||||
f"platform '{platform}' references toolset '{name}' "
|
||||
"which is not available on this platform"
|
||||
)
|
||||
continue
|
||||
suggestion = _platform_default_toolset(platform)
|
||||
hint = (
|
||||
f" — did you mean '{suggestion}'?"
|
||||
if is_valid_toolset(suggestion)
|
||||
if _platform_default_is_valid(
|
||||
platform,
|
||||
suggestion,
|
||||
is_valid_toolset,
|
||||
is_allowed_for_platform,
|
||||
)
|
||||
else ""
|
||||
)
|
||||
warnings.append(
|
||||
@@ -84,6 +146,16 @@ def validate_platform_toolsets(
|
||||
f"'{name}'{hint}"
|
||||
)
|
||||
|
||||
if platform_valid_count == 0:
|
||||
if isinstance(raw, list) and not raw:
|
||||
reason = "is configured with an empty toolset list"
|
||||
else:
|
||||
reason = "has no valid toolsets configured"
|
||||
warnings.append(
|
||||
f"platform '{platform}' {reason} — the agent will have no "
|
||||
f"tools on this platform. Run `hermes tools` to reconfigure."
|
||||
)
|
||||
|
||||
if valid_count == 0:
|
||||
warnings.append(
|
||||
"platform_toolsets resolves to zero valid toolsets — the agent will "
|
||||
|
||||
@@ -74,6 +74,30 @@ def test_partially_valid_platform_toolsets_no_runtime_warning(caplog):
|
||||
assert not any("#38798" in r.getMessage() for r in caplog.records)
|
||||
|
||||
|
||||
def test_null_platform_toolsets_fall_back_to_platform_default():
|
||||
"""A YAML ``platform:`` value is absent, not an explicit empty list."""
|
||||
config = {"platform_toolsets": {"cli": None}}
|
||||
|
||||
enabled = _get_platform_tools(config, "cli", include_default_mcp_servers=False)
|
||||
default_enabled = _get_platform_tools(
|
||||
{}, "cli", include_default_mcp_servers=False
|
||||
)
|
||||
|
||||
assert enabled == default_enabled
|
||||
|
||||
|
||||
def test_scalar_platform_toolsets_fall_back_to_platform_default():
|
||||
"""A non-list platform value is ignored by the resolver."""
|
||||
config = {"platform_toolsets": {"cli": "bogus"}}
|
||||
|
||||
enabled = _get_platform_tools(config, "cli", include_default_mcp_servers=False)
|
||||
default_enabled = _get_platform_tools(
|
||||
{}, "cli", include_default_mcp_servers=False
|
||||
)
|
||||
|
||||
assert enabled == default_enabled
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
@@ -14,6 +14,8 @@ _KNOWN = {
|
||||
"hermes-cli",
|
||||
"hermes-telegram",
|
||||
"hermes-discord",
|
||||
"hermes-whatsapp",
|
||||
"discord",
|
||||
"terminal",
|
||||
"web",
|
||||
}
|
||||
@@ -41,9 +43,14 @@ def test_mixed_valid_and_invalid_flags_only_the_invalid():
|
||||
warnings = validate_platform_toolsets(cfg, _is_valid)
|
||||
# One valid entry exists, so no zero-valid warning.
|
||||
assert not any("zero valid toolsets" in w for w in warnings)
|
||||
assert len(warnings) == 1
|
||||
assert "platform 'discord'" in warnings[0]
|
||||
assert "unknown toolset 'bogus'" in warnings[0]
|
||||
assert any(
|
||||
"platform 'discord'" in w and "unknown toolset 'bogus'" in w
|
||||
for w in warnings
|
||||
)
|
||||
assert any(
|
||||
"platform 'discord'" in w and "no valid toolsets" in w
|
||||
for w in warnings
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -94,6 +101,98 @@ def test_empty_list_does_not_mask_unknown_names_on_other_platforms():
|
||||
assert any("zero valid toolsets" in w for w in warnings)
|
||||
|
||||
|
||||
def test_null_platform_warns_even_when_others_are_valid():
|
||||
# YAML's ``cli:`` parses as None. The resolver treats it as absent and uses
|
||||
# the platform default, so the warning must not claim that tools disappear.
|
||||
cfg = {"cli": None, "telegram": ["hermes-telegram"]}
|
||||
warnings = validate_platform_toolsets(cfg, _is_valid)
|
||||
|
||||
assert any(
|
||||
"platform 'cli'" in w
|
||||
and "null toolset value" in w
|
||||
and "falling back to 'hermes-cli'" in w
|
||||
for w in warnings
|
||||
)
|
||||
assert not any("zero valid toolsets" in w for w in warnings)
|
||||
|
||||
|
||||
def test_null_platform_uses_canonical_default_for_alias():
|
||||
cfg = {"whatsapp_cloud": None, "telegram": ["hermes-telegram"]}
|
||||
warnings = validate_platform_toolsets(cfg, _is_valid)
|
||||
|
||||
assert any(
|
||||
"platform 'whatsapp_cloud'" in w
|
||||
and "falling back to 'hermes-whatsapp'" in w
|
||||
for w in warnings
|
||||
)
|
||||
assert not any("zero valid toolsets" in w for w in warnings)
|
||||
|
||||
|
||||
def test_scalar_platform_value_warns_but_uses_platform_default():
|
||||
cfg = {"cli": "bogus", "telegram": ["hermes-telegram"]}
|
||||
warnings = validate_platform_toolsets(cfg, _is_valid)
|
||||
|
||||
assert any(
|
||||
"platform 'cli'" in w
|
||||
and "invalid toolset value 'bogus'" in w
|
||||
and "falling back to 'hermes-cli'" in w
|
||||
for w in warnings
|
||||
)
|
||||
assert not any("zero valid toolsets" in w for w in warnings)
|
||||
|
||||
|
||||
def test_platform_restricted_toolset_warns_when_other_platform_is_valid():
|
||||
cfg = {"telegram": ["discord"], "cli": ["hermes-cli"]}
|
||||
warnings = validate_platform_toolsets(cfg, _is_valid)
|
||||
|
||||
assert any(
|
||||
"platform 'telegram'" in w
|
||||
and "toolset 'discord'" in w
|
||||
and "not available" in w
|
||||
for w in warnings
|
||||
)
|
||||
assert not any("zero valid toolsets" in w for w in warnings)
|
||||
|
||||
|
||||
def test_null_plugin_platform_uses_synthetic_default():
|
||||
from gateway.platform_registry import PlatformEntry, platform_registry
|
||||
from toolsets import resolve_toolset
|
||||
|
||||
platform = "toolset_validation_plugin"
|
||||
platform_registry.register(
|
||||
PlatformEntry(
|
||||
name=platform,
|
||||
label="Toolset Validation Plugin",
|
||||
adapter_factory=lambda _config: object(),
|
||||
check_fn=lambda: True,
|
||||
)
|
||||
)
|
||||
try:
|
||||
cfg = {platform: None, "telegram": ["hermes-telegram"]}
|
||||
warnings = validate_platform_toolsets(cfg, _is_valid)
|
||||
|
||||
assert resolve_toolset(f"hermes-{platform}")
|
||||
assert any(
|
||||
f"platform '{platform}'" in w
|
||||
and f"falling back to 'hermes-{platform}'" in w
|
||||
for w in warnings
|
||||
)
|
||||
assert not any("zero valid toolsets" in w for w in warnings)
|
||||
finally:
|
||||
platform_registry.unregister(platform)
|
||||
|
||||
|
||||
def test_all_invalid_platform_warns_even_when_others_are_valid():
|
||||
cfg = {"cli": ["bogus"], "telegram": ["hermes-telegram"]}
|
||||
warnings = validate_platform_toolsets(cfg, _is_valid)
|
||||
|
||||
assert any("unknown toolset 'bogus'" in w for w in warnings)
|
||||
assert any(
|
||||
"platform 'cli'" in w and "no valid toolsets" in w for w in warnings
|
||||
)
|
||||
assert not any("zero valid toolsets" in w for w in warnings)
|
||||
|
||||
|
||||
def test_populated_platforms_produce_no_empty_list_warning():
|
||||
cfg = {"cli": ["hermes-cli"], "telegram": ["hermes-telegram"]}
|
||||
warnings = validate_platform_toolsets(cfg, _is_valid)
|
||||
|
||||
Reference in New Issue
Block a user