From 5317993a6dca17f2269cbf79bff88cc912ba92a8 Mon Sep 17 00:00:00 2001 From: Nick Mason Date: Wed, 1 Jul 2026 11:09:57 -0400 Subject: [PATCH] fix(tools): expose static (pre-registry-merge) toolset view for platform inference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- tests/test_toolsets.py | 38 +++++++++++++++++++++++++++ toolsets.py | 58 +++++++++++++++++++++++++++++++----------- 2 files changed, 81 insertions(+), 15 deletions(-) diff --git a/tests/test_toolsets.py b/tests/test_toolsets.py index 1773d281a..f9e4969b7 100644 --- a/tests/test_toolsets.py +++ b/tests/test_toolsets.py @@ -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) == [] diff --git a/toolsets.py b/toolsets.py index 083ab9d89..af1ee4a92 100644 --- a/toolsets.py +++ b/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-). # 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)