Skip to content

feat(mcp): per-platform MCP server scoping via mcp_servers.<name>.platforms - #125393

Open
Finn763 wants to merge 1 commit into
NousResearch:mainfrom
Finn763:fix/110916-mcp-platforms
Open

Finn763 wants to merge 1 commit into
NousResearch:mainfrom
Finn763:fix/110916-mcp-platforms

Conversation

@Finn763

@Finn763 Finn763 commented Sep 27, 2026 •

Copy link
Copy Markdown

Closes #110916

Adds an optional server-side allowlist: mcp_servers..platforms (e.g. [discord]). Servers scoped to other platforms are excluded from _get_platform_tools resolution for that platform, even when explicitly listed in its platform_toolsets (server-side scope wins). Absent/null platforms means every platform (backward compatible); unparseable values fail open with a warning.

Enforcement point: enabled_mcp_server_names(config, platform) (new optional param, default keeps legacy set) threaded through _merge_mcp_servers from _get_platform_tools, so CLI/gateway/api_server/ACP/discord all resolve through the one chokepoint. MCP tool definitions and dispatch both derive from enabled_toolsets, so the scope holds at both seams.

Verification:

  • RED on pristine main: scoped server leaks into api_server toolsets (failing test run, 110916-red.log).
  • GREEN: new test test_mcp_server_platforms_scopes_server_to_listed_platforms passes; neighbors tests/hermes_cli/test_tools_config.py + tests/tools/test_mcp_enabled_reader.py + tests/cron/test_scheduler.py: 159 passed, 6 skipped (110916-neigh.log).

Known ceiling: portable plugin servers (in-memory, no config entry) cannot be scoped. The remaining direct enabled_mcp_server_names callers without a platform are diagnostics/membership checks (cron preflight; the ACP membership intersection, which intersects the already-filtered set and cannot re-admit). The MCP connection pool is process-wide by design (a multiplexed gateway shares one connection across platforms), so per-platform exposure is enforced where toolsets are assembled: _get_platform_tools(<platform_key>), which gateway/CLI/TUI/ACP/api_server/cron all route through. Cron's per-job MCP merge now threads platform=cron too, so a scoped-away server cannot reach a job by name.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard tool/mcp MCP client and OAuth area/config Config system, migrations, profiles labels Sep 27, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated follow-up for reference; not a maintainer.

Per-platform MCP scoping: mcp_server_allowed_for_platform() (tools/mcp_tool_common.py:164-175), a
platform param on enabled_mcp_server_names() (hermes_cli/tools_config.py:428-441), plumbed through
_merge_mcp_servers() / _get_platform_tools(). Precedence over platform_toolsets is correct.

1. platforms: ["*"] silently scopes the server OUT of every platform — no wildcard, no warning.
tools/mcp_tool_common.py:170-174 returns str(platform or "").strip().lower() in allowed, with no
"*" / "all" handling. Deterministic run of the post-image body:

key absent / null   discord  ALLOWED
platforms: []       discord  *** EXCLUDED ***
platforms: ["*"]    discord  *** EXCLUDED ***

A user migrating a global server writes platforms: ["*"] meaning "all platforms" → allowed = {"*"}
→ "discord" not in {"*"} → False → dropped from _merge_mcp_servers()'s allowed set on every
platform, with no warning at all. The only symptom: a tool that silently
disappears, and ["*"] behaves exactly like []. Fix: treat {"*", "all", "any"} as "serve every
platform", and warn when a listed platform matches no known platform, so a typo like ["dscord"] isn't
indistinguishable from a real one.

2. The guard is wired into toolset resolution only; the connection path never consults it (please confirm).
mcp_server_allowed_for_platform has one call site — hermes_cli/tools_config.py:437-440. The four other
callers (cron/scheduler.py, acp_adapter/session.py, tui_gateway/…) pass no platform and get the
unfiltered set your docstring documents. But the layer that actually connects gates only on
mcp_server_enabled() (tools/mcp_tool_registration.py:524,537,547, hermes_cli/tools_config_mcp.py:88)
and never calls mcp_server_allowed_for_platform. If those runtimes pass a raw mcp_servers mapping
rather than the filtered set from _get_platform_tools(), a server scoped away from the current platform
is still connectable by name — a visibility filter rather than an access-control boundary.

Unverified / please confirm: finding 1 is measured (table = running the post-image body) but not via
the real hermes_cli.tools_config import (no usable dependency env here), so its effect on
_get_platform_tools() is reasoned, not executed. Finding 2 is code-read only; the registration path
was not probed. Test read, not run.

…tforms

Servers scoped to other platforms are excluded from _get_platform_tools
resolution even when explicitly listed (server-side scope wins); absent
platforms = all (compat). '*'/'all'/'any' mean every platform, and a
scope naming no known platform is warned about once instead of silently
disabling the server everywhere. Cron's per-job MCP merge now follows
the same cron-platform rules, so a server scoped away from cron cannot
reach a job by being named there either.

Closes NousResearch#110916
@Finn763
Finn763 force-pushed the fix/110916-mcp-platforms branch from 07c1464 to 94666df Compare October 6, 2026 14:58
@Finn763

Finn763 commented Oct 6, 2026

Copy link
Copy Markdown
Author

Both points addressed at head 94666dfee45d (rebased onto main 85db7c3a).

1. platforms: ["*"] silently excluding the server — fixed. {"*", "all", "any"} now mean every platform, so a migrated global server keeps working; an explicit [] keeps meaning "no platform". A scope that names no known platform now warns once per distinct list (MCP 'platforms' scope ['dscord'] contains unknown name(s) ...) instead of silently disabling the server everywhere. New tests: test_mcp_server_platforms_wildcard_serves_every_platform, test_mcp_server_platforms_typo_warns_and_stays_excluded.

2. Connection path — confirmed, traced, and one real leak closed. The MCP client pool is process-wide and shared across platforms/profiles by design (a multiplexed gateway connects a server once and serves several platforms from one connection), so the connection layer cannot be the per-platform gate without breaking that sharing. The enforcement point is toolset assembly: every runtime building an agent's toolset routes through _get_platform_tools(config, <platform_key>) — gateway (platform_key), CLI ("cli"), ACP ("acp"), TUI ("cli"), api_server ("api_server"), cron default ("cron") — and with this PR that resolver excludes scoped-away servers even when named. One genuine leak of this class did exist: cron's per-job enabled_toolsets merge (_merge_mcp_into_per_job_toolsets) unioned the unfiltered enabled set and kept a named scoped-away server. It now uses enabled_mcp_server_names(cfg, "cron") and drops scoped-out names — new test test_platform_scoped_server_never_reaches_a_cron_job. Left as-is (not exposure paths): cron/scheduler_preflight.py is a diagnostic, and the ACP membership check is an intersection with the already-filtered resolved, so it cannot re-admit a server.

Tests: focused → 10 passed; full neighbor files (tests/hermes_cli/test_tools_config.py, tests/cron/test_scheduler.py, tests/tools/test_mcp_enabled_reader.py) → 163 passed, 6 skipped; ruff check clean on all touched files.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have tool/mcp MCP client and OAuth type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: per-platform MCP server scoping

3 participants