fix(acp): admit config MCP servers by platform_toolsets.acp like the gateway
A fresh ACP agent appended mcp-<server> 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-<server>. 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.
This commit is contained in:
@@ -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-<server>`` 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),
|
||||
|
||||
@@ -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.<platform>``,
|
||||
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
|
||||
|
||||
@@ -119,7 +119,8 @@ Examples:
|
||||
```text
|
||||
new_session(cwd)
|
||||
-> create SessionState
|
||||
-> create AIAgent(platform="acp", enabled_toolsets=<platform_toolsets.acp, default hermes-acp>,
|
||||
-> create AIAgent(platform="acp", enabled_toolsets=<platform_toolsets.acp, default hermes-acp,
|
||||
plus mcp-<server> for the config MCP servers it admits>,
|
||||
disabled_toolsets=<agent.disabled_toolsets>)
|
||||
-> bind task_id/session_id to cwd override
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user