From 316d6fb8f76daa95810d5218c15415e950f44fdc Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 23 Sep 2026 09:12:55 -0700 Subject: [PATCH] fix(acp): admit config MCP servers by platform_toolsets.acp like the gateway A fresh ACP agent appended mcp- for every enabled config MCP server unconditionally, so a platform_toolsets.acp allowlist of server names and the no_mcp sentinel were ignored on ACP while the gateway honoured both. The MCP half now comes from the same _get_platform_tools(config, "acp") call as the base toolsets: its server names (default every enabled server, a listed allowlist, or none for no_mcp) are keyed as mcp-. Editor-provided session/new servers are unchanged. Docs: `hermes tools` has no ACP platform entry, so drop the claim that it configures platform_toolsets.acp; document the MCP rules with a config example. --- acp_adapter/session.py | 20 ++++++------- tests/acp_adapter/test_session.py | 30 +++++++++++++++++++ website/docs/developer-guide/acp-internals.md | 3 +- website/docs/user-guide/features/acp.md | 20 +++++++++++-- 4 files changed, 58 insertions(+), 15 deletions(-) diff --git a/acp_adapter/session.py b/acp_adapter/session.py index 064e7b46f2..9598f18af6 100644 --- a/acp_adapter/session.py +++ b/acp_adapter/session.py @@ -466,7 +466,7 @@ class SessionManager: from agent.skill_utils import parse_config_string_list from hermes_cli.config import load_config from hermes_cli.runtime_provider import resolve_runtime_provider - from hermes_cli.tools_config import _get_platform_tools + from hermes_cli.tools_config import _get_platform_tools, enabled_mcp_server_names from hermes_constants import resolve_reasoning_config config = load_config() @@ -477,18 +477,16 @@ class SessionManager: elif isinstance(model_cfg, str): default_model = model_cfg.strip() - from tools.mcp_tool_common import mcp_server_enabled - - configured_mcp_servers = [ - name for name, cfg in (config.get("mcp_servers") or {}).items() - if not isinstance(cfg, dict) or mcp_server_enabled(cfg) - ] + if enabled_toolsets is None: + # The same per-platform resolver as the gateway/cron/api_server: platform_toolsets.acp wins, else + # hermes-acp; its MCP half (every enabled server, a listed-name allowlist, or none for ``no_mcp``) + # comes back as bare server names, which ACP keys as ``mcp-`` like its session servers. + resolved = _get_platform_tools(config, "acp") + mcp_servers = resolved & enabled_mcp_server_names(config) + enabled_toolsets = _expand_acp_enabled_toolsets(sorted(resolved - mcp_servers), sorted(mcp_servers)) kwargs = { "platform": "acp", "quiet_mode": True, "session_id": session_id, "session_db": self._get_db(), - # The same per-platform resolver as the gateway/cron/api_server: platform_toolsets.acp wins, else hermes-acp. - "enabled_toolsets": (list(enabled_toolsets) if enabled_toolsets is not None else _expand_acp_enabled_toolsets( - sorted(_get_platform_tools(config, "acp", include_default_mcp_servers=False)), - mcp_server_names=configured_mcp_servers)), + "enabled_toolsets": list(enabled_toolsets), # agent.disabled_toolsets is subtracted at tool granularity by the agent, as on the CLI/gateway/cron. "disabled_toolsets": (list(disabled_toolsets) if disabled_toolsets is not None else parse_config_string_list((config.get("agent") or {}).get("disabled_toolsets")) or None), diff --git a/tests/acp_adapter/test_session.py b/tests/acp_adapter/test_session.py index ede5ca677b..0c61026389 100644 --- a/tests/acp_adapter/test_session.py +++ b/tests/acp_adapter/test_session.py @@ -173,6 +173,36 @@ class TestCreateSession: assert set(resolve_toolset(offered)) <= names assert not names & set(resolve_toolset(withheld)) + @pytest.mark.parametrize("acp_toolsets, expected_mcp", [ + (None, {"mcp-alpha", "mcp-beta"}), # default: every enabled config server + (["hermes-acp", "alpha"], {"mcp-alpha"}), # listed server names are an allowlist + (["hermes-acp", "no_mcp"], set()), # the no_mcp sentinel drops them all + ]) + def test_fresh_agent_mcp_servers_follow_platform_toolsets(self, monkeypatch, acp_toolsets, expected_mcp): + """Config MCP servers reach a fresh ACP agent by the gateway's rules for ``platform_toolsets.``, + not unconditionally; a disabled server never does.""" + seen: list[dict] = [] + + class FakeAgent: + def __init__(self, **kwargs): + seen.append(kwargs) + + config = {"model": {"default": "m"}, + "mcp_servers": {"alpha": {"command": "a"}, "beta": {"command": "b"}, "off": {"enabled": False}}} + if acp_toolsets is not None: + config["platform_toolsets"] = {"acp": acp_toolsets} + monkeypatch.setattr("run_agent.AIAgent", FakeAgent) + monkeypatch.setattr("hermes_cli.config.load_config", lambda: config) + monkeypatch.setattr("hermes_cli.runtime_provider.resolve_runtime_provider", lambda **_kw: {}) + monkeypatch.setattr("hermes_cli.mcp_startup.ensure_mcp_discovery_before_agent_build", lambda **_kw: None) + monkeypatch.setattr("acp_adapter.session._register_task_cwd", lambda task_id, cwd: None) + + SessionManager(db=None)._make_agent(session_id="fresh", cwd=".") + + enabled = seen[0]["enabled_toolsets"] + assert {t for t in enabled if t.startswith("mcp-")} == expected_mcp + assert not {"alpha", "beta", "no_mcp"} & set(enabled) + def test_make_agent_surfaces_the_provider_resolution_failure(self, monkeypatch): """#91090: when ``resolve_runtime_provider`` fails, the bare-AIAgent fallback dies with the first-run "No LLM provider configured" text; the operator must get the swallowed cause diff --git a/website/docs/developer-guide/acp-internals.md b/website/docs/developer-guide/acp-internals.md index fee4bea103..9e1b6811f8 100644 --- a/website/docs/developer-guide/acp-internals.md +++ b/website/docs/developer-guide/acp-internals.md @@ -119,7 +119,8 @@ Examples: ```text new_session(cwd) -> create SessionState - -> create AIAgent(platform="acp", enabled_toolsets=, + -> create AIAgent(platform="acp", enabled_toolsets= for the config MCP servers it admits>, disabled_toolsets=) -> bind task_id/session_id to cwd override diff --git a/website/docs/user-guide/features/acp.md b/website/docs/user-guide/features/acp.md index 6c5fbfbbbe..cc2d340b8e 100644 --- a/website/docs/user-guide/features/acp.md +++ b/website/docs/user-guide/features/acp.md @@ -35,9 +35,23 @@ Hermes runs with a curated `hermes-acp` toolset designed for editor workflows. I It intentionally excludes things that do not fit typical editor UX, such as messaging delivery and cronjob management. -The toolset resolves like every other platform: `platform_toolsets.acp` in -`config.yaml` (or `hermes tools`) replaces the `hermes-acp` default, and -`agent.disabled_toolsets` removes toolsets from every ACP session. +The toolset resolves the same way as on the messaging gateway. +`platform_toolsets.acp` replaces the `hermes-acp` default, and +`agent.disabled_toolsets` removes toolsets from every ACP session. MCP +servers from `mcp_servers` follow the same rules too. By default ACP gets +every enabled server. If you list server names in `platform_toolsets.acp`, +only those servers are included, and `no_mcp` drops them all. `hermes tools` +has no ACP entry, so edit `config.yaml` directly: + +```yaml +platform_toolsets: + acp: [file, web, skills, github] # only the github MCP server +agent: + disabled_toolsets: [code_execution] +``` + +MCP servers that the editor sends with `session/new` are separate. The +client asks for them per session, and they are always added. ## Installation