fix(tools): don't drop a toolset from platform inference when a tool is registered into it
_get_platform_tools reverse-maps a platform composite to configurable toolsets with an all-tools subset test. Because get_toolset() merges registry-registered tools into a toolset, a tool added to a toolset (delegate_cli -> delegation; desktop-only read_terminal -> terminal) that the static composite never listed made the subset test fail, silently dropping the entire toolset on api_server and other inference-based platforms. Compare the toolset's static membership at all three reverse-map sites. Fixes #49622. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
5317993a6d
commit
80733413f9
2 changed files with 67 additions and 3 deletions
|
|
@ -1471,7 +1471,11 @@ def _get_platform_tools(
|
|||
for ts_key, _, _ in CONFIGURABLE_TOOLSETS:
|
||||
if not _toolset_allowed_for_platform(ts_key, platform):
|
||||
continue
|
||||
ts_tools = set(resolve_toolset(ts_key))
|
||||
# Compare the toolset's STATIC membership: a tool registered
|
||||
# into a toolset (e.g. delegate_cli -> delegation, desktop-only
|
||||
# read_terminal -> terminal) that the composite never listed must
|
||||
# not drop the whole toolset. See issue #49622.
|
||||
ts_tools = set(resolve_toolset(ts_key, include_registry=False))
|
||||
if ts_tools and ts_tools.issubset(composite_tools):
|
||||
expanded.add(ts_key)
|
||||
|
||||
|
|
@ -1494,7 +1498,12 @@ def _get_platform_tools(
|
|||
for ts_key, _, _ in CONFIGURABLE_TOOLSETS:
|
||||
if not _toolset_allowed_for_platform(ts_key, platform):
|
||||
continue
|
||||
ts_tools = set(resolve_toolset(ts_key))
|
||||
# Compare the toolset's STATIC membership against the composite (see
|
||||
# issue #49622): get_toolset() merges registry-registered tools into
|
||||
# a toolset, but platform composites enumerate static tool names, so
|
||||
# an all-tools subset test against the merged set drops the whole
|
||||
# toolset the moment a plugin/overlay/desktop tool joins it.
|
||||
ts_tools = set(resolve_toolset(ts_key, include_registry=False))
|
||||
if ts_tools and ts_tools.issubset(all_tool_names):
|
||||
enabled_toolsets.add(ts_key)
|
||||
|
||||
|
|
@ -1566,7 +1575,10 @@ def _get_platform_tools(
|
|||
# by agent/coding_context.py — not per-platform capabilities to recover.
|
||||
if ts_def.get("posture"):
|
||||
continue
|
||||
ts_tools = set(resolve_toolset(ts_key))
|
||||
# Static membership (see #49622): a registry-added tool absent from the
|
||||
# platform composite must not block recovery of a non-configurable
|
||||
# toolset whose authored tools the composite does list.
|
||||
ts_tools = set(resolve_toolset(ts_key, include_registry=False))
|
||||
if not ts_tools or not ts_tools.issubset(platform_tool_universe):
|
||||
continue
|
||||
if ts_tools.issubset(configurable_tool_universe):
|
||||
|
|
|
|||
|
|
@ -63,6 +63,58 @@ class TestApiServerPlatformConfig:
|
|||
assert "api_server" in PLATFORMS
|
||||
assert PLATFORMS["api_server"]["default_toolset"] == "hermes-api-server"
|
||||
|
||||
def test_default_api_server_includes_terminal_toolset(self):
|
||||
"""Regression #49622: desktop-only read_terminal is registered into the
|
||||
'terminal' toolset (ships in-repo), so resolve_toolset('terminal') grows
|
||||
to include it after discovery. read_terminal is NOT in the
|
||||
hermes-api-server composite, so the old all-tools subset test dropped
|
||||
'terminal' entirely. Its static membership (terminal, process) IS in the
|
||||
composite, so it must stay enabled."""
|
||||
from tools.registry import discover_builtin_tools
|
||||
from hermes_cli.tools_config import _get_platform_tools
|
||||
discover_builtin_tools()
|
||||
assert "terminal" in _get_platform_tools({}, "api_server")
|
||||
|
||||
def test_registering_tool_into_toolset_does_not_drop_toolset_from_inference(self):
|
||||
"""Class invariant (covers the delegate_cli overlay case): registering a
|
||||
NEW tool into an existing configurable toolset must never remove that
|
||||
toolset from a platform whose composite lists the toolset's static
|
||||
tools. Synthetic registration keeps the test hermetic in CI."""
|
||||
from tools.registry import registry
|
||||
from hermes_cli.tools_config import _get_platform_tools
|
||||
|
||||
sentinel = "test_sentinel_delegation_tool"
|
||||
registry.register(
|
||||
name=sentinel,
|
||||
toolset="delegation",
|
||||
schema={"name": sentinel, "description": "test",
|
||||
"parameters": {"type": "object", "properties": {}}},
|
||||
handler=lambda args, **kw: "{}",
|
||||
)
|
||||
try:
|
||||
# delegation's static membership (delegate_task) is in the composite,
|
||||
# so the toolset must survive inference despite the extra registry tool.
|
||||
assert "delegation" in _get_platform_tools({}, "api_server"), (
|
||||
"registering a tool into 'delegation' dropped it from api_server"
|
||||
)
|
||||
finally:
|
||||
registry.deregister(sentinel)
|
||||
|
||||
def test_default_off_and_restricted_toolsets_stay_off_on_api_server(self):
|
||||
"""Negative contract: the static-membership comparison must NOT newly
|
||||
enable default-off or platform-restricted toolsets."""
|
||||
import os
|
||||
from unittest.mock import patch
|
||||
from hermes_cli.tools_config import _get_platform_tools
|
||||
with patch.dict(os.environ, {}, clear=False):
|
||||
os.environ.pop("HASS_TOKEN", None)
|
||||
os.environ.pop("XAI_API_KEY", None)
|
||||
enabled = _get_platform_tools({}, "api_server")
|
||||
assert "homeassistant" not in enabled
|
||||
assert "discord" not in enabled
|
||||
assert "discord_admin" not in enabled
|
||||
assert "x_search" not in enabled
|
||||
|
||||
|
||||
class TestApiServerAdapterToolset:
|
||||
@patch("gateway.platforms.api_server.AIOHTTP_AVAILABLE", True)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue