Skip to content
Open
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
15 changes: 12 additions & 3 deletions cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -2743,9 +2743,18 @@ def _init_toolsets(self, toolsets):
self.disabled_toolsets = parse_config_string_list(CLI_CONFIG["agent"].get("disabled_toolsets"))

if toolsets and "all" not in toolsets and "*" not in toolsets:
# MCP server names only resolve after discover_mcp_tools runs; skip them here.
mcp_names = set((CLI_CONFIG.get("mcp_servers") or {}).keys())
invalid = [t for t in toolsets if not validate_toolset(t) and t not in mcp_names]
# Validate each toolset β€” MCP server toolsets are registered as
# ``mcp-<server-name>`` aliases during discover_mcp_tools(), but
# discovery hasn't run yet at this point, so exclude configured MCP
# aliases from the early static warning.
from hermes_cli.mcp_toolsets import mcp_toolset_aliases_for_servers

mcp_aliases = mcp_toolset_aliases_for_servers(CLI_CONFIG.get("mcp_servers"))
invalid = [
t
for t in toolsets
if not validate_toolset(t) and t not in mcp_aliases
]
if invalid:
self._console_print(f"[bold red]Warning: Unknown toolsets: {', '.join(invalid)}[/]")

Expand Down
77 changes: 77 additions & 0 deletions hermes_cli/mcp_toolsets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
"""Helpers for configured MCP server toolset names.

MCP discovery registers runtime toolsets as ``mcp-<server>`` and also exposes
bare server-name aliases. Several startup paths validate explicit toolset lists
before discovery has run, so they need a config-only view of both spellings.
"""

from __future__ import annotations

from collections.abc import Mapping


def _parse_enabled_flag(value: object, default: bool = True) -> bool:
"""Parse bool-like config values used by MCP server settings."""
if value is None:
return default
if isinstance(value, bool):
return value
if isinstance(value, int):
return value != 0
if isinstance(value, str):
lowered = value.strip().lower()
if lowered in {"true", "1", "yes", "on"}:
return True
if lowered in {"false", "0", "no", "off"}:
return False
return default


def _server_enabled(server_cfg: object) -> bool:
"""Return whether an MCP server config is enabled by config semantics."""
if isinstance(server_cfg, Mapping):
return _parse_enabled_flag(server_cfg.get("enabled", True), default=True)
return True


def mcp_toolset_aliases_for_servers(
mcp_servers: object,
*,
enabled: bool | None = None,
) -> set[str]:
"""Return configured MCP toolset spellings for startup validation.

Args:
mcp_servers: The ``config.yaml`` ``mcp_servers`` mapping.
enabled: ``True`` for only enabled servers, ``False`` for only disabled
servers, ``None`` for every configured server.

Each configured server contributes both accepted toolset spellings:
``<server>`` and ``mcp-<server>``. The latter is the runtime toolset name
registered by MCP discovery; the former is the historical alias supported
by earlier startup validators.
"""
Comment thread
thedavidweng marked this conversation as resolved.
if not isinstance(mcp_servers, Mapping):
return set()

aliases: set[str] = set()
for name, server_cfg in mcp_servers.items():
if not isinstance(server_cfg, Mapping):
continue
name = str(name)
is_enabled = _server_enabled(server_cfg)
if enabled is not None and is_enabled is not enabled:
continue
aliases.add(name)
aliases.add(f"mcp-{name}")
return aliases


def split_configured_mcp_toolset_aliases(
mcp_servers: object,
) -> tuple[set[str], set[str]]:
"""Return ``(enabled_aliases, disabled_aliases)`` for configured MCP servers."""
return (
mcp_toolset_aliases_for_servers(mcp_servers, enabled=True),
mcp_toolset_aliases_for_servers(mcp_servers, enabled=False),
)
39 changes: 16 additions & 23 deletions hermes_cli/oneshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,26 +68,6 @@ def _build_preloaded_skills_prompt(skills: object = None) -> str | None:
return skills_prompt or None


def _configured_mcp_servers() -> tuple[set[str], set[str]]:
"""``(enabled, disabled)`` MCP server names from config; both empty on any error."""
try:
from hermes_cli.config import read_raw_config
from hermes_cli.tools_config import _parse_enabled_flag

cfg = read_raw_config()
mcp_servers = cfg.get("mcp_servers") if isinstance(cfg.get("mcp_servers"), dict) else {}
enabled: set[str] = set()
disabled: set[str] = set()
for name, server_cfg in mcp_servers.items():
if not isinstance(server_cfg, dict):
continue
target = enabled if _parse_enabled_flag(server_cfg.get("enabled", True), default=True) else disabled
target.add(str(name))
return enabled, disabled
except Exception:
return set(), set()


def _validate_explicit_toolsets(toolsets: object = None) -> tuple[list[str] | None, str | None]:
normalized = _normalize_toolsets(toolsets)
if normalized is None:
Expand Down Expand Up @@ -121,10 +101,23 @@ def _validate_explicit_toolsets(toolsets: object = None) -> tuple[list[str] | No
)
return None, None

mcp_names, mcp_disabled = _configured_mcp_servers() if unresolved else (set(), set())
mcp_valid = [name for name in unresolved if name in mcp_names]
mcp_aliases: set[str] = set()
mcp_disabled: set[str] = set()
if unresolved:
try:
from hermes_cli.config import read_raw_config
from hermes_cli.mcp_toolsets import split_configured_mcp_toolset_aliases

cfg = read_raw_config()
mcp_servers = cfg.get("mcp_servers") if isinstance(cfg.get("mcp_servers"), dict) else {}
mcp_aliases, mcp_disabled = split_configured_mcp_toolset_aliases(mcp_servers)
except Exception:
mcp_aliases = set()
mcp_disabled = set()

mcp_valid = [name for name in unresolved if name in mcp_aliases]
disabled = [name for name in unresolved if name in mcp_disabled]
unknown = [name for name in unresolved if name not in mcp_names and name not in mcp_disabled]
unknown = [name for name in unresolved if name not in mcp_aliases and name not in mcp_disabled]
valid = built_in + mcp_valid

if unknown:
Expand Down
74 changes: 74 additions & 0 deletions tests/cli/test_mcp_toolset_validation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
from copy import deepcopy

import cli


def _init_cli_and_capture_warnings(monkeypatch, toolsets):
cfg = deepcopy(cli.CLI_CONFIG)
cfg["model"] = {"default": "stub-model", "provider": "auto", "base_url": ""}
cfg.setdefault("agent", {})["disabled_toolsets"] = []
cfg.setdefault("display", {})
cfg["mcp_servers"] = {
"agentmail": {"enabled": True},
"notion-findit": {"enabled": True},
}

warnings = []
monkeypatch.setattr(cli, "CLI_CONFIG", cfg)
monkeypatch.setattr(cli, "validate_toolset", lambda name: name == "terminal")
monkeypatch.setattr(
cli.HermesCLI,
"_console_print",
lambda self, message, *args, **kwargs: warnings.append(str(message)),
)

cli.HermesCLI(
model="stub-model",
provider="auto",
toolsets=toolsets,
compact=True,
)
return warnings


def test_cli_startup_accepts_mcp_prefixed_toolsets_before_discovery(monkeypatch):
warnings = _init_cli_and_capture_warnings(
monkeypatch,
["terminal", "mcp-agentmail", "mcp-notion-findit"],
)

assert warnings == []


def test_cli_startup_still_warns_for_non_mcp_unknown_toolsets(monkeypatch):
warnings = _init_cli_and_capture_warnings(
monkeypatch,
["terminal", "mcp-agentmail", "not-real"],
)

assert warnings == ["[bold red]Warning: Unknown toolsets: not-real[/]"]


def test_mcp_toolset_aliases_skips_non_mapping_entries():
"""Non-mapping server entries are skipped to avoid validating toolsets
that discovery cannot register."""
from hermes_cli.mcp_toolsets import mcp_toolset_aliases_for_servers

result = mcp_toolset_aliases_for_servers({
"good": {"enabled": True},
"bad": "just-a-string",
"also-bad": 42,
})
assert result == {"good", "mcp-good"}


def test_split_configured_mcp_toolset_aliases_skips_non_mapping():
from hermes_cli.mcp_toolsets import split_configured_mcp_toolset_aliases

enabled, disabled = split_configured_mcp_toolset_aliases({
"active": {"enabled": True},
"inactive": {"enabled": False},
"malformed": "not-a-dict",
})
assert enabled == {"active", "mcp-active"}
assert disabled == {"inactive", "mcp-inactive"}
Loading