Merge pull request #89819 from NousResearch/bb/keep-gui-surface-direct
fix(tool-search): keep GUI surface tools directly available (supersedes #88029)
This commit is contained in:
@@ -85,6 +85,67 @@ class TestClassification:
|
||||
for name in BRIDGE_TOOL_NAMES:
|
||||
assert not is_deferrable_tool_name(name)
|
||||
|
||||
def test_gui_surface_tools_never_defer(self):
|
||||
"""Session-gated GUI tools stay direct and stay off the global core list."""
|
||||
from tools.registry import discover_builtin_tools
|
||||
from tools.tool_search import is_deferrable_tool_name
|
||||
from toolsets import _HERMES_CORE_TOOLS
|
||||
|
||||
discover_builtin_tools()
|
||||
for name in ("read_window_below", "apply_layout", "project_list"):
|
||||
assert not is_deferrable_tool_name(name), name
|
||||
assert name not in _HERMES_CORE_TOOLS
|
||||
|
||||
def test_gui_surface_alone_does_not_activate_the_bridge(self):
|
||||
from tools.registry import discover_builtin_tools
|
||||
from tools.tool_search import ToolSearchConfig, assemble_tool_defs
|
||||
|
||||
discover_builtin_tools()
|
||||
names = {"read_window_below", "apply_layout", "project_list"}
|
||||
assembled = assemble_tool_defs(
|
||||
[_td(name, f"GUI {name}") for name in names],
|
||||
context_length=200_000,
|
||||
config=ToolSearchConfig.from_raw({"enabled": "on"}),
|
||||
)
|
||||
assert not assembled.activated
|
||||
assert {td["function"]["name"] for td in assembled.tool_defs} == names
|
||||
|
||||
def test_gui_surface_stays_direct_when_mcp_activates_the_bridge(self):
|
||||
"""MCP/plugin tools turn Tool Search on; the session's GUI tools stay
|
||||
in the model-facing array so HUD can still name read_window_below."""
|
||||
from tools.registry import discover_builtin_tools, registry
|
||||
from tools.tool_search import (
|
||||
BRIDGE_TOOL_NAMES,
|
||||
ToolSearchConfig,
|
||||
assemble_tool_defs,
|
||||
)
|
||||
|
||||
discover_builtin_tools()
|
||||
mcp_name = "mcp_gui_surface_probe"
|
||||
registry.register(
|
||||
name=mcp_name,
|
||||
handler=lambda args, **kw: "{}",
|
||||
schema=_td(mcp_name, "Deferred MCP capability")["function"],
|
||||
toolset="mcp-gui-surface-probe",
|
||||
)
|
||||
|
||||
assembled = assemble_tool_defs(
|
||||
[
|
||||
_td("read_window_below", "Identify the window below"),
|
||||
_td("apply_layout", "Apply a layout preset"),
|
||||
_td("computer_use", "Drive the OS"),
|
||||
_td(mcp_name, "Deferred MCP capability"),
|
||||
],
|
||||
context_length=200_000,
|
||||
config=ToolSearchConfig.from_raw({"enabled": "on"}),
|
||||
)
|
||||
names = {td["function"]["name"] for td in assembled.tool_defs}
|
||||
|
||||
assert assembled.activated
|
||||
assert mcp_name not in names
|
||||
assert BRIDGE_TOOL_NAMES <= names
|
||||
assert {"read_window_below", "apply_layout", "computer_use"} <= names
|
||||
|
||||
def test_unknown_tool_not_deferrable(self):
|
||||
"""Defensive: a tool name we cannot resolve to a registry entry must
|
||||
not be claimed as deferrable. This protects against the OpenClaw
|
||||
|
||||
@@ -22,6 +22,17 @@ from tui_gateway import server
|
||||
FULL_KIT = {"read_window_below", "computer_use", "browser_navigate"}
|
||||
|
||||
|
||||
def _tool_def(name: str) -> dict:
|
||||
return {
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": name,
|
||||
"description": name,
|
||||
"parameters": {"type": "object", "properties": {}},
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def _session(*, tools=FULL_KIT, **extra):
|
||||
return {
|
||||
"agent": types.SimpleNamespace(valid_tool_names=set(tools)),
|
||||
@@ -82,6 +93,34 @@ class TestTurnRouting:
|
||||
|
||||
assert server._hud_surface_note(session) == ""
|
||||
|
||||
def test_note_survives_tool_search_when_mcp_tools_are_present(self):
|
||||
"""valid_tool_names is the post-assembly list. MCP tools used to hide
|
||||
read_window_below there, so a HUD turn got no note at all."""
|
||||
from tools.registry import discover_builtin_tools, registry
|
||||
from tools.tool_search import ToolSearchConfig, assemble_tool_defs
|
||||
|
||||
discover_builtin_tools()
|
||||
mcp_name = "mcp_hud_surface_probe"
|
||||
registry.register(
|
||||
name=mcp_name,
|
||||
handler=lambda args, **kw: "{}",
|
||||
schema=_tool_def(mcp_name)["function"],
|
||||
toolset="mcp-hud-surface-probe",
|
||||
)
|
||||
|
||||
assembled = assemble_tool_defs(
|
||||
[_tool_def(name) for name in (*FULL_KIT, mcp_name)],
|
||||
context_length=200_000,
|
||||
config=ToolSearchConfig.from_raw({"enabled": "on"}),
|
||||
)
|
||||
names = {td["function"]["name"] for td in assembled.tool_defs}
|
||||
|
||||
assert assembled.activated
|
||||
assert mcp_name not in names
|
||||
assert server._hud_surface_note(_session(tools=names, client_surface="hud")) == (
|
||||
hud_surface_note(FULL_KIT)
|
||||
)
|
||||
|
||||
|
||||
class TestPrepending:
|
||||
"""Notes ride the model input; the persisted prompt stays what was typed."""
|
||||
|
||||
+17
-4
@@ -9,6 +9,11 @@ for the full rationale):
|
||||
|
||||
* Core tools defined in ``toolsets._HERMES_CORE_TOOLS`` are *never* deferred.
|
||||
Always-load means always-load. No exceptions.
|
||||
* Session-gated GUI toolsets (``desktop_ui``, ``project``) are also never
|
||||
deferred. They stay off the core list so CLI and messaging never pay for
|
||||
their schemas, but once a session enables them they stay in the
|
||||
model-facing array. Tool Search is for MCP/plugin catalog bloat, not for
|
||||
hiding the tools that define this session's surface.
|
||||
* Tiered disclosure (July 2026 plan): the moment ANY deferrable (MCP/plugin)
|
||||
tools are present, they hide behind the bridge. What scales with catalog
|
||||
size is the *listing*, not the activation decision:
|
||||
@@ -201,12 +206,18 @@ def _core_tool_names() -> frozenset[str]:
|
||||
return frozenset()
|
||||
|
||||
|
||||
# Session-gated GUI toolsets. Off ``_HERMES_CORE_TOOLS`` so non-GUI clients
|
||||
# never pay their schema; once a session enables them they stay direct.
|
||||
_DIRECT_SURFACE_TOOLSETS = frozenset({"desktop_ui", "project"})
|
||||
|
||||
|
||||
def is_deferrable_tool_name(name: str) -> bool:
|
||||
"""Return True if a tool with this name is *eligible* for deferral.
|
||||
|
||||
A tool is deferrable iff it is registered with an MCP toolset prefix
|
||||
OR it is not in ``_HERMES_CORE_TOOLS``. Core tools are never deferred
|
||||
even when their toolset is technically plugin-provided (this protects
|
||||
OR it is neither in ``_HERMES_CORE_TOOLS`` nor a session-gated GUI
|
||||
surface toolset. Core and direct surface tools are never deferred even
|
||||
when their toolset is technically plugin-provided (this protects
|
||||
against accidental shadowing).
|
||||
"""
|
||||
if name in BRIDGE_TOOL_NAMES:
|
||||
@@ -221,6 +232,8 @@ def is_deferrable_tool_name(name: str) -> bool:
|
||||
return False
|
||||
if entry.toolset.startswith("mcp-"):
|
||||
return True
|
||||
if entry.toolset in _DIRECT_SURFACE_TOOLSETS:
|
||||
return False
|
||||
# Non-MCP, non-core → plugin tool, eligible.
|
||||
return True
|
||||
except Exception:
|
||||
@@ -231,8 +244,8 @@ def classify_tools(tool_defs: List[Dict[str, Any]]) -> Tuple[List[Dict[str, Any]
|
||||
"""Split a tool-defs list into (visible, deferrable).
|
||||
|
||||
``visible`` retains every tool that must stay in the model-facing array:
|
||||
every core tool, plus any tool we can't classify. ``deferrable`` is the
|
||||
candidate set for catalog entry.
|
||||
every core tool, every session-gated GUI surface tool, plus any tool we
|
||||
can't classify. ``deferrable`` is the candidate set for catalog entry.
|
||||
"""
|
||||
visible: List[Dict[str, Any]] = []
|
||||
deferrable: List[Dict[str, Any]] = []
|
||||
|
||||
Reference in New Issue
Block a user