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
11 changes: 9 additions & 2 deletions cron/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -472,13 +472,20 @@ def _resolve_cron_disabled_toolsets(cfg: dict) -> list[str]:
def _merge_mcp_into_per_job_toolsets(per_job: list[str], cfg: dict) -> list[str]:
"""Layer enabled MCP servers onto a per-job ``enabled_toolsets`` allowlist (else a per-job list
silently drops every MCP server). Mirrors ``_get_platform_tools``: ``no_mcp`` sentinel -> none
(stripped); any MCP server already listed -> allowlist, add nothing; else union all enabled."""
(stripped); any MCP server already listed -> allowlist, add nothing; else union all enabled.
Servers scoped via ``mcp_servers.<name>.platforms`` follow the same platform rules as
``_get_platform_tools(cfg, "cron")``: one scoped to other platforms is dropped even when the
job names it explicitly (server-side scope wins), and is never auto-added."""
result = [t for t in per_job if t != "no_mcp"]
if "no_mcp" in per_job:
return result
# lazy: avoid heavy hermes_cli import at module load; shares MCP-membership with gateway/CLI
from hermes_cli.tools_config import enabled_mcp_server_names
enabled_mcp = enabled_mcp_server_names(cfg)
globally_enabled = enabled_mcp_server_names(cfg)
enabled_mcp = enabled_mcp_server_names(cfg, "cron")
scoped_out = globally_enabled - enabled_mcp
if scoped_out:
result = [t for t in result if t not in scoped_out]
if set(result) & enabled_mcp:
return result
for name in sorted(enabled_mcp):
Expand Down
26 changes: 16 additions & 10 deletions hermes_cli/tools_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,16 +407,18 @@ def _parse_enabled_flag(value, default: bool = True) -> bool:
return default


def enabled_mcp_server_names(config: dict) -> Set[str]:
def enabled_mcp_server_names(config: dict, platform: Optional[str] = None) -> Set[str]:
"""MCP servers globally enabled in config.yaml or by a plugin (shared by platform + cron resolvers). Enabled
unless ``enabled`` is explicitly falsey; portable-plugin servers (in-memory) count — enabling the plugin is
the opt-in."""
from tools.mcp_tool_common import mcp_server_enabled
the opt-in. With *platform*, servers scoped via ``mcp_servers.<name>.platforms`` to other platforms are
excluded (#110916); ``None`` keeps the legacy unfiltered set."""
from tools.mcp_tool_common import mcp_server_allowed_for_platform, mcp_server_enabled

mcp_servers = (config or {}).get("mcp_servers") or {}
names = {
str(name) for name, server_cfg in mcp_servers.items()
if isinstance(server_cfg, dict) and mcp_server_enabled(server_cfg)
and (platform is None or mcp_server_allowed_for_platform(server_cfg, platform))
}
try:
from hermes_cli.plugins import get_portable_mcp_server_names_nowait
Expand Down Expand Up @@ -619,7 +621,7 @@ def _get_platform_tools(config: dict, platform: str, *, include_default_mcp_serv

# Explicit non-configurable entries (custom toolsets, MCP server names) pass through.
explicit_passthrough = {ts for ts in toolset_names if ts not in explicit_known_keys and ts not in platform_default_keys}
enabled_toolsets |= _merge_mcp_servers(config, toolset_names, explicit_passthrough, include_default_mcp_servers)
enabled_toolsets |= _merge_mcp_servers(config, toolset_names, explicit_passthrough, include_default_mcp_servers, platform)

# Legacy profile opt-in is a fallback only. A saved platform list (even
# empty) is authoritative, so a later disable cannot silently re-enable it.
Expand Down Expand Up @@ -683,17 +685,21 @@ def _recover_platform_native_toolsets(enabled_toolsets: Set[str], platform: str,


def _merge_mcp_servers(
config: dict, toolset_names: List[str], explicit_passthrough: Set[str], include_default_mcp_servers: bool
config: dict, toolset_names: List[str], explicit_passthrough: Set[str], include_default_mcp_servers: bool,
platform: Optional[str] = None,
) -> Set[str]:
"""Explicit passthrough entries plus this platform's MCP servers: listed names form an allowlist, else every
globally enabled server (when ``include_default_mcp_servers``); the ``no_mcp`` sentinel disables all."""
enabled_mcp_servers = enabled_mcp_server_names(config)
result = explicit_passthrough - enabled_mcp_servers
globally enabled server (when ``include_default_mcp_servers``); the ``no_mcp`` sentinel disables all.
A server scoped via ``mcp_servers.<name>.platforms`` to another platform is excluded even when explicitly
listed (#110916: server-side scope wins over the per-platform list)."""
globally_enabled = enabled_mcp_server_names(config)
allowed = enabled_mcp_server_names(config, platform) if platform else globally_enabled
result = explicit_passthrough - globally_enabled
if "no_mcp" in toolset_names:
return result - {"no_mcp"}
explicit_mcp_servers = explicit_passthrough & enabled_mcp_servers
explicit_mcp_servers = explicit_passthrough & allowed
if include_default_mcp_servers and not explicit_mcp_servers:
return result | enabled_mcp_servers
return result | allowed
return result | explicit_mcp_servers


Expand Down
15 changes: 15 additions & 0 deletions tests/cron/test_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,21 @@ def test_explicit_mcp_name_is_treated_as_allowlist(self):
assert result == ["web", "finnhub"]
assert "playwright" not in result

def test_platform_scoped_server_never_reaches_a_cron_job(self):
# platforms: [discord] = server-side scope wins: not auto-added, and dropped even
# when the job names it explicitly (parity with _get_platform_tools).
cfg = {
"mcp_servers": {
"discord_admin": {"enabled": True, "platforms": ["discord"]},
"finnhub": {"enabled": True},
}
}
merged = _merge_mcp_into_per_job_toolsets(["web"], cfg)
assert "finnhub" in merged and "discord_admin" not in merged
named = _merge_mcp_into_per_job_toolsets(["web", "discord_admin"], cfg)
assert "discord_admin" not in named
assert "finnhub" in named

def test_no_mcp_sentinel_opts_out_and_is_stripped(self):
result = _merge_mcp_into_per_job_toolsets(["web", "no_mcp"], self.CFG)
assert result == ["web"]
Expand Down
40 changes: 40 additions & 0 deletions tests/hermes_cli/test_tools_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,46 @@ def test_numeric_mcp_server_name_does_not_crash_sorted():
sorted(enabled)


def test_mcp_server_platforms_scopes_server_to_listed_platforms():
"""https://github.com/NousResearch/hermes-agent/issues/110916: a server with
``mcp_servers.<name>.platforms`` must not leak to other platforms, even when
explicitly listed in their ``platform_toolsets``; unscoped servers are unaffected."""
config = {
"mcp_servers": {
"discord_admin": {"command": "x", "platforms": ["discord"]},
"open_srv": {"command": "x"},
},
}

api = _get_platform_tools(config, "api_server")
assert "discord_admin" not in api
assert "open_srv" in api
discord = _get_platform_tools(config, "discord")
assert "discord_admin" in discord

explicit = dict(config)
explicit["platform_toolsets"] = {"api_server": ["discord_admin"]}
assert "discord_admin" not in _get_platform_tools(explicit, "api_server")


def test_mcp_server_platforms_wildcard_serves_every_platform():
"""``["*"]`` (and its all/any aliases) means every platform — not a literal platform name."""
for token in ("*", "all", "any"):
config = {"mcp_servers": {"everywhere": {"command": "x", "platforms": [token]}}}
for platform in ("discord", "api_server", "cron"):
assert "everywhere" in _get_platform_tools(config, platform), (token, platform)


def test_mcp_server_platforms_typo_warns_and_stays_excluded(caplog):
"""A scope naming no known platform (e.g. a typo) must warn once instead of silently
disabling the server everywhere."""
config = {"mcp_servers": {"typo_srv": {"command": "x", "platforms": ["dscord"]}}}
with caplog.at_level(logging.WARNING, logger="tools.mcp_tool"):
assert "typo_srv" not in _get_platform_tools(config, "discord")
warnings = [record.getMessage() for record in caplog.records]
assert any("dscord" in message for message in warnings), warnings


# ─── Imagegen Backend Picker Wiring ────────────────────────────────────────


Expand Down
54 changes: 54 additions & 0 deletions tools/mcp_tool_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,60 @@ def mcp_server_enabled(cfg: dict) -> bool:
return _parse_boolish(cfg.get("enabled", True), default=True)


#: ``platforms`` list entries that mean "serve every platform" rather than a platform name.
_ALL_PLATFORM_TOKENS = frozenset({"*", "all", "any"})
#: Warn once per distinct scope list: resolution runs per turn, and a config typo would
#: otherwise warn on every one of them.
_WARNED_UNKNOWN_PLATFORM_SCOPES: set = set()


def _known_platform_names() -> set:
"""Platform keys a ``platforms`` scope can name: gateway platforms (incl. bundled plugin
adapters) plus the non-messaging runtimes that resolve toolsets through ``_get_platform_tools``
(CLI, cron, ACP). Best-effort — used for typo warnings only."""
known: set = {"cli", "cron", "acp"}
try:
from gateway.config import Platform

known.update(member.value for member in Platform)
try:
bundled, _aliases = Platform._scan_bundled_plugin_platforms()
known.update(bundled)
except Exception:
pass
except Exception:
pass
return known


def mcp_server_allowed_for_platform(cfg: dict, platform: str) -> bool:
"""Whether ``mcp_servers.<name>`` may serve *platform*. Absent/``null`` ``platforms`` = every
platform (backward compatible); a list scopes the server to those platforms only (#110916).
``*``/``all``/``any`` in the list mean every platform; a listed name matching no known
platform is warned about (once per distinct list) instead of silently disabling the server."""
raw = cfg.get("platforms", None)
if raw is None:
return True
if isinstance(raw, str):
raw = [raw]
if not isinstance(raw, (list, tuple, set)):
logger.warning("MCP config expected a list for 'platforms', got %r; ignoring", raw)
return True
allowed = {str(p).strip().lower() for p in raw if str(p).strip()}
if allowed & _ALL_PLATFORM_TOKENS:
return True
known = _known_platform_names()
unknown = allowed - known
if unknown and frozenset(allowed) not in _WARNED_UNKNOWN_PLATFORM_SCOPES:
_WARNED_UNKNOWN_PLATFORM_SCOPES.add(frozenset(allowed))
logger.warning(
"MCP 'platforms' scope %s contains unknown name(s) %s (known: %s); use '*', "
"'all' or 'any' to serve every platform",
sorted(allowed), sorted(unknown), ", ".join(sorted(known)),
)
return str(platform or "").strip().lower() in allowed


def _get_lifecycle_seconds(config: dict, key: str) -> Optional[float]:
"""Optional positive lifecycle timeout from top-level/nested ``lifecycle`` config (``0``
disables; negatives and non-numbers are warned about and ignored)."""
Expand Down