fix(tools): expose static (pre-registry-merge) toolset view for platform inference
Adds include_registry=True kwarg to resolve_toolset/get_toolset. When False, returns only the static TOOLSETS view with no registry-merged tools — the composite-authored membership platform reverse-mapping must compare against. Default True preserves all existing behavior; this is the enabling half of the api_server toolset-drop fix (#49622). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
6a58badfdc
commit
5317993a6d
2 changed files with 81 additions and 15 deletions
|
|
@ -253,3 +253,41 @@ class TestDefaultPlatformWebSearchCoverage:
|
|||
|
||||
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) == []
|
||||
|
|
|
|||
58
toolsets.py
58
toolsets.py
|
|
@ -583,19 +583,39 @@ TOOLSETS = {
|
|||
|
||||
|
||||
|
||||
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 (registry-only toolsets have no static view,
|
||||
so they return None when include_registry=False)
|
||||
"""
|
||||
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:
|
||||
|
|
@ -662,30 +682,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)
|
||||
|
||||
|
|
@ -698,12 +724,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
|
||||
|
|
@ -730,9 +758,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)
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue