Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions hermes_cli/tools_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -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):
Expand Down
3 changes: 1 addition & 2 deletions scripts/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@

# Auto-extracted from noreply emails + manual overrides
AUTHOR_MAP = {
"jvsantos.cunha@gmail.com": "plcunha", # PR #55300 salvage (gateway: record child gateway peer metadata after a compression session-id rotation and repoint stale sessions.json compression-parent entries to the recovered live child; consolidated in the compression-routing-integrity salvage)
"jakepresent1@gmail.com": "jakepresent", # PR #55721 salvage (gateway: identity-guard stale in-flight compression splits — a late run may publish its compressed child only if its run generation is still current and the session key still points at the run's original parent, so an old run can't overwrite a newer /new or moved binding)
"zhangml@tech.icbc.com.cn": "zmlgit", # PR #54872 salvage (multiplex-profile kanban: route task notifications via the owning profile's adapter + wake the creator agent with a synthetic internal MessageEvent on terminal events)
"1079826437@qq.com": "nankingjing", # PR #56404 salvage (gateway: while a state.db compression lock is held for the session, demote busy_input_mode 'interrupt' to 'queue' so a rapid message burst can't interrupt and fork orphaned compression siblings off a stale parent; #56391)
"ud@arubangles.com": "udatny", # PR #29433 salvage (subdirectory_hints: catch RuntimeError from Path.expanduser()/Path.home() so a literal ~ in tool-call args — e.g. LLM "~500-700" or ~unknownuser — can't escape the hint walker and crash the conversation loop)
Expand Down Expand Up @@ -1836,6 +1834,7 @@
"max.petrusenko.agent@gmail.com": "maxpetrusenko", # PR #54128 co-author
"poli.koltsova@gmail.com": "wnuuee1", # commit 9fd2b2cb PR author
"yosapol@jitrak.dev": "Eji4h", # direct email match
"kiljadn@gmail.com": "designnotdrum", # PR #56480 salvage (toolset static-inference fix)
}


Expand Down
52 changes: 52 additions & 0 deletions tests/gateway/test_api_server_toolset.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,58 @@ def test_platforms_dict_includes_api_server(self):
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)
Expand Down
38 changes: 38 additions & 0 deletions tests/test_toolsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,3 +253,41 @@ def test_hermes_whatsapp_toolset_includes_web_search(self):

def test_hermes_api_server_toolset_includes_web_search(self):
assert "web_search" in resolve_toolset("hermes-api-server")


class TestResolveToolsetIncludeRegistry:
"""include_registry flag exposes the static (pre-registry-merge) view used
by platform reverse-mapping. Regression harness for issue #49622."""

def test_include_registry_false_excludes_registry_tools(self):
from tools.registry import discover_builtin_tools
discover_builtin_tools() # registers read_terminal into 'terminal'

merged = set(resolve_toolset("terminal"))
static = set(resolve_toolset("terminal", include_registry=False))

assert static == {"terminal", "process"}, static
# read_terminal is registered into 'terminal' but is desktop-only and
# not part of the static definition — it must only appear in the merged view.
assert "read_terminal" in merged
assert "read_terminal" not in static

def test_get_toolset_include_registry_false_is_static(self):
ts = get_toolset("delegation", include_registry=False)
assert ts is not None
assert ts["tools"] == ["delegate_task"]

def test_static_view_threads_through_includes(self):
# 'debugging' has direct tools [terminal, process] and includes [web, file]
static = set(resolve_toolset("debugging", include_registry=False))
assert {"terminal", "process"} <= static
assert "web_search" in static
assert "read_file" in static

def test_all_alias_accepts_include_registry(self):
merged = set(resolve_toolset("all"))
static = set(resolve_toolset("all", include_registry=False))
assert static <= merged

def test_registry_only_toolset_static_view_is_empty(self):
assert resolve_toolset("__definitely_not_a_real_toolset__", include_registry=False) == []
60 changes: 45 additions & 15 deletions toolsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -583,19 +583,41 @@



def get_toolset(name: str) -> Optional[Dict[str, Any]]:
def get_toolset(name: str, *, include_registry: bool = True) -> Optional[Dict[str, Any]]:
"""
Get a toolset definition by name.

Args:
name (str): Name of the toolset

include_registry (bool): When True (default), merge in tools that
plugins/overlays registered into this toolset via the registry.
When False, return only the static ``TOOLSETS`` definition (the
composite-authored view). Platform reverse-mapping in
``_get_platform_tools`` uses False so that a tool registered into a
toolset but absent from a platform's static composite does not drop
the whole toolset from inference. See issue #49622.

Returns:
Dict: Toolset definition with description, tools, and includes
None: If toolset not found
None: If toolset not found. With include_registry=False the static
view only recognizes names literally present in ``TOOLSETS``, so
registry/MCP-only toolsets AND registry-derived aliases return None
(they have no static counterpart).
"""
toolset = TOOLSETS.get(name)

if not include_registry:
# Static view only: return the built-in definition (copying the nested
# tools/includes lists so callers can't mutate TOOLSETS), or None for
# registry/MCP-only toolsets that have no static counterpart.
if not toolset:
return None
return {
**toolset,
"tools": list(toolset.get("tools", [])),
"includes": list(toolset.get("includes", [])),
}

try:
from tools.registry import registry
except Exception:
Expand Down Expand Up @@ -662,30 +684,36 @@ def bundle_non_core_tools(toolset_name: str) -> Set[str]:
return to_remove


def resolve_toolset(name: str, visited: Set[str] = None) -> List[str]:
def resolve_toolset(name: str, visited: Set[str] = None, *, include_registry: bool = True) -> List[str]:
"""
Recursively resolve a toolset to get all tool names.

This function handles toolset composition by recursively resolving
included toolsets and combining all tools.

Args:
name (str): Name of the toolset to resolve
visited (Set[str]): Set of already visited toolsets (for cycle detection)

include_registry (bool): When True (default), include tools that
plugins/overlays registered into a toolset. When False, resolve only
the static ``TOOLSETS`` definition (includes are still resolved, but
statically). Platform reverse-mapping uses False so a registry-added
tool cannot drop the whole toolset from inference (see #49622 and
``_get_platform_tools``).

Returns:
List[str]: List of all tool names in the toolset
"""
if visited is None:
visited = set()

# Special aliases that represent all tools across every toolset
# This ensures future toolsets are automatically included without changes.
if name in {"all", "*"}:
all_tools: Set[str] = set()
for toolset_name in get_toolset_names():
# Use a fresh visited set per branch to avoid cross-branch contamination
resolved = resolve_toolset(toolset_name, visited.copy())
resolved = resolve_toolset(toolset_name, visited.copy(), include_registry=include_registry)
all_tools.update(resolved)
return sorted(all_tools)

Expand All @@ -698,12 +726,14 @@ def resolve_toolset(name: str, visited: Set[str] = None) -> List[str]:
visited.add(name)

# Get toolset definition
toolset = get_toolset(name)
toolset = get_toolset(name, include_registry=include_registry)
if not toolset:
# Auto-generate a toolset for plugin platforms (hermes-<name>).
# Gives them _HERMES_CORE_TOOLS plus any tools the plugin registered
# into a toolset matching the platform name.
if name.startswith("hermes-"):
# into a toolset matching the platform name. This is a registry-derived
# view, so it only applies when registry tools are requested; the static
# view (include_registry=False) has no plugin-platform definition.
if include_registry and name.startswith("hermes-"):
platform_name = name[len("hermes-"):]
try:
from gateway.platform_registry import platform_registry
Expand All @@ -730,9 +760,9 @@ def resolve_toolset(name: str, visited: Set[str] = None) -> List[str]:
# sibling includes so diamond dependencies are only resolved once and
# cycle warnings don't fire multiple times for the same cycle.
for included_name in toolset.get("includes", []):
included_tools = resolve_toolset(included_name, visited)
included_tools = resolve_toolset(included_name, visited, include_registry=include_registry)
tools.update(included_tools)

return sorted(tools)


Expand Down
Loading