diff --git a/acp_adapter/commands.py b/acp_adapter/commands.py index 4d9a606824..522140d7f7 100644 --- a/acp_adapter/commands.py +++ b/acp_adapter/commands.py @@ -156,7 +156,7 @@ class SlashCommandsMixin: from types import SimpleNamespace from agent.memory_manager import inject_memory_provider_tools - toolsets = _expand_acp_enabled_toolsets(getattr(state.agent, "enabled_toolsets", None) or ["hermes-acp"]) + toolsets = _expand_acp_enabled_toolsets(getattr(state.agent, "enabled_toolsets", None)) tools = get_tool_definitions( enabled_toolsets=toolsets, disabled_toolsets=getattr(state.agent, "disabled_toolsets", None), diff --git a/acp_adapter/server.py b/acp_adapter/server.py index f3fb441118..d23e358999 100644 --- a/acp_adapter/server.py +++ b/acp_adapter/server.py @@ -447,7 +447,7 @@ class HermesACPAgent(SlashCommandsMixin, acp.Agent): agent = state.agent agent.enabled_toolsets = _expand_acp_enabled_toolsets( - getattr(agent, "enabled_toolsets", None) or ["hermes-acp"], + getattr(agent, "enabled_toolsets", None), mcp_server_names=[s.name for s in mcp_servers], ) agent.tools = get_tool_definitions( diff --git a/acp_adapter/session.py b/acp_adapter/session.py index 2e9a179a06..064e7b46f2 100644 --- a/acp_adapter/session.py +++ b/acp_adapter/session.py @@ -103,7 +103,7 @@ def _register_task_cwd(task_id: str, cwd: str) -> None: def _expand_acp_enabled_toolsets(toolsets: List[str] | None = None, mcp_server_names: List[str] | None = None) -> List[str]: """Return ACP toolsets plus explicit MCP server toolsets for this session.""" - names = [n for n in (toolsets or ["hermes-acp"]) if n] + names = [n for n in (["hermes-acp"] if toolsets is None else toolsets) if n] names += [f"mcp-{s}" for s in (mcp_server_names or []) if s] return list(dict.fromkeys(names)) @@ -458,7 +458,7 @@ class SessionManager: requested_provider: str | None = None, base_url: str | None = None, api_mode: str | None = None, enabled_toolsets: list[str] | None = None, disabled_toolsets: list[str] | None = None): """``enabled_toolsets``/``disabled_toolsets`` carry a live session's toolsets into a rebuild; ``None`` derives - them from the config-declared MCP servers (fresh session).""" + them from config (fresh session).""" if self._agent_factory is not None: return self._agent_factory() @@ -466,6 +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_constants import resolve_reasoning_config config = load_config() @@ -484,8 +485,10 @@ class SessionManager: ] kwargs = { "platform": "acp", "quiet_mode": True, "session_id": session_id, "session_db": self._get_db(), - "enabled_toolsets": (list(enabled_toolsets) if enabled_toolsets is not None - else _expand_acp_enabled_toolsets(["hermes-acp"], mcp_server_names=configured_mcp_servers)), + # 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)), # 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 f4e0c1c129..ede5ca677b 100644 --- a/tests/acp_adapter/test_session.py +++ b/tests/acp_adapter/test_session.py @@ -135,9 +135,44 @@ class TestCreateSession: session_id="rebuilt", cwd=".", enabled_toolsets=["hermes-acp", "mcp-acp-server"], disabled_toolsets=["browser"], ) - assert (seen[0]["enabled_toolsets"], seen[0]["disabled_toolsets"]) == (["hermes-acp", "mcp-cfg-server"], None) + assert "mcp-cfg-server" in seen[0]["enabled_toolsets"] and seen[0]["disabled_toolsets"] is None assert (seen[1]["enabled_toolsets"], seen[1]["disabled_toolsets"]) == (["hermes-acp", "mcp-acp-server"], ["browser"]) + @pytest.mark.parametrize("config, offered, withheld", [ + # agent.disabled_toolsets, in the JSON-string shape `hermes config set` stores (#74582). + ({"agent": {"disabled_toolsets": "['code_execution']"}}, "file", "code_execution"), + # platform_toolsets.acp narrows the surface like every other platform (#79516). + ({"platform_toolsets": {"acp": ["file"]}}, "file", "code_execution"), + ]) + def test_fresh_agent_tool_surface_honours_toolset_config(self, monkeypatch, config, offered, withheld): + """A fresh ACP agent resolves its tools like the gateway/cron: the real tool surface built from its + kwargs carries the offered toolset and none of the withheld one.""" + from model_tools import get_tool_definitions + from toolsets import resolve_toolset + + seen: list[dict] = [] + + class FakeAgent: + def __init__(self, **kwargs): + seen.append(kwargs) + + monkeypatch.setattr("run_agent.AIAgent", FakeAgent) + monkeypatch.setattr("hermes_cli.config.load_config", lambda: {"model": {"default": "m"}, **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=".") + + def surface(enabled, disabled=None) -> set: + return {t["function"]["name"] for t in get_tool_definitions( + enabled_toolsets=enabled, disabled_toolsets=disabled, quiet_mode=True)} + + assert set(resolve_toolset(withheld)) <= surface(["hermes-acp"]) # non-vacuous: offered by default + names = surface(seen[0]["enabled_toolsets"], seen[0]["disabled_toolsets"]) + assert set(resolve_toolset(offered)) <= names + assert not names & set(resolve_toolset(withheld)) + 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 0445b323ab..fee4bea103 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=["hermes-acp"]) + -> create AIAgent(platform="acp", enabled_toolsets=, + disabled_toolsets=) -> bind task_id/session_id to cwd override prompt(..., session_id) diff --git a/website/docs/user-guide/features/acp.md b/website/docs/user-guide/features/acp.md index 8d7ad9dd47..6c5fbfbbbe 100644 --- a/website/docs/user-guide/features/acp.md +++ b/website/docs/user-guide/features/acp.md @@ -35,6 +35,10 @@ 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. + ## Installation Install Hermes normally, then add the ACP extra from the install checkout: @@ -268,12 +272,10 @@ therefore runs shell commands on the host without prompting. I asked one to run Selecting `Anyone` hands that same shell access to every author who can reach the channel. Buzz does not warn when you pick it. -Neither of the obvious mitigations works today: - -- `approvals.mode: manual` does make Hermes raise the permission request, but - Buzz auto-approves it and the command still runs. -- `platform_toolsets.acp` does not narrow the ACP toolset, so it cannot be used - to drop `terminal`. +`approvals.mode: manual` does not help: Hermes raises the permission request, +but Buzz auto-approves it and the command still runs. To take the shell away, +narrow the toolset instead: set `platform_toolsets.acp` to a list without +`terminal` and `code_execution`, or add them to `agent.disabled_toolsets`. `!shutdown` from the owner stops the agent in any mode, and Buzz ignores that command from everyone else.