""" Tests for the unified builtin tool registry (POLICIES.md §15.8). Phase 2 unification: `BUILTIN_NAMES` and the instantiable subset both derive from a single `_BUILTIN_REGISTRY` dict, with `None` factories marking framework-owned names (``web_fetch``). These tests lock the name-space invariants so any future registry change has to either deliberately touch them or break loudly. """ from __future__ import annotations import importlib import pkgutil import omnigent.tools.builtins as _builtins_pkg from omnigent.tools.base import Tool from omnigent.tools.builtins import ( BUILTIN_NAMES, INSTANTIABLE_BUILTINS, get_builtin_tool, ) def test_builtin_names_excludes_request_approval() -> None: """``request_approval`` is no longer reserved. The synthetic function_call by that name was deleted when policy ASKs moved to MCP-shape elicitations (``response.elicitation_request`` SSE event + session ``approval`` event). User specs are now free to declare a tool called ``request_approval`` — no carve-out, no collision. A regression where ``request_approval`` reappears in BUILTIN_NAMES would silently re-reserve a name we no longer own; this test guards that. """ assert "request_approval" not in BUILTIN_NAMES def test_builtin_names_includes_framework_owned_tools() -> None: """web_fetch, list_comments, and update_comment are framework-owned (need runtime context, not instantiated via the registry). They must still occupy the name-space so user specs can't declare tools with these names.""" assert "web_fetch" in BUILTIN_NAMES assert "list_comments" in BUILTIN_NAMES assert "update_comment" in BUILTIN_NAMES assert "sys_list_models" in BUILTIN_NAMES def test_instantiable_subset_excludes_framework_owned() -> None: """Framework-owned names are NOT in INSTANTIABLE_BUILTINS because they have no factory. The onboarding assistant uses this set to tell the agent author what they can declare — listing framework-owned names there would be confusing and wrong.""" assert "web_fetch" not in INSTANTIABLE_BUILTINS assert "list_comments" not in INSTANTIABLE_BUILTINS assert "update_comment" not in INSTANTIABLE_BUILTINS def test_instantiable_is_subset_of_builtin_names() -> None: """Every instantiable name is also a reserved name. The two sets can't get out of sync because they derive from the same dict — this test guards against a refactor that introduces drift.""" # subset check expressed via issubset — clearer than # "for-in" iteration. assert INSTANTIABLE_BUILTINS.issubset(BUILTIN_NAMES) def test_get_builtin_tool_returns_none_for_framework_owned() -> None: """Calling get_builtin_tool on a framework-owned name returns None — the caller must fall back to the special constructor path. This is the same behavior as an unknown name, which is fine because BUILTIN_NAMES is the authoritative "is this reserved?" set.""" assert get_builtin_tool("web_fetch") is None assert get_builtin_tool("list_comments") is None assert get_builtin_tool("update_comment") is None def test_get_builtin_tool_returns_none_for_unknown_name() -> None: """Unknown names also return None. Callers that want to distinguish "unknown" from "framework-owned" must check `name in BUILTIN_NAMES` first.""" assert get_builtin_tool("definitely_not_a_tool") is None def test_get_builtin_tool_instantiates_known_tools() -> None: """Instantiable tools produce a real Tool instance. Smoke test — if this breaks, every agent with `web_search` declared starts failing at load time.""" tool = get_builtin_tool("web_search") # Not None + correct name — proves both the factory ran # and produced an instance with the expected identity. assert tool is not None assert tool.name() == "web_search" def test_builtin_names_size_matches_registry() -> None: """A sanity check that the derivation is lossless. If someone adds a new registry entry but BUILTIN_NAMES doesn't reflect it (impossible under current derivation, but a refactor could miss it), this test turns red.""" # Lock the expected set so adding / removing a name is an # explicit test edit. assert ( frozenset( { # Instantiable "web_search", "upload_file", "list_files", "download_file", "search_conversations", "export_agent", # Hindsight long-term memory tools (optional `hindsight` # extra; factories probe for hindsight-client). "hindsight_retain", "hindsight_recall", "hindsight_reflect", # Framework-owned (need runtime context, not # user-instantiable). Policy ASKs surface as # MCP-shape elicitations on the SSE stream and # do NOT reserve a name in this registry. # The ``sys_terminal_*`` family also lives outside # this registry — registered by ToolManager when # the spec declares ``terminals:``. "web_fetch", # Comment tools: auto-registered by ToolManager so # agents can list/update review comments without # spec opt-in. Session-scoped at invoke time via # ToolContext.conversation_id. "list_comments", "update_comment", # sys_list_models / sys_advise_models: auto-registered # by ToolManager alongside the sub-agent dispatch grant. # sys_advise_models is only included when smart routing # is enabled (RuntimeCaps.routing_client is set). "sys_list_models", "sys_advise_models", # Embedded-browser tools: always registered (schema-only # Tool classes; execution is runner-dispatched via the # _BROWSER_TOOLS branch in runner/tool_dispatch.py). "browser_navigate", "browser_snapshot", "browser_click", "browser_type", "browser_screenshot", } ) == BUILTIN_NAMES ) def _all_builtin_tool_subclasses() -> list[type[Tool]]: """Concrete ``Tool`` subclasses defined under ``omnigent.tools.builtins``.""" for mod_info in pkgutil.iter_modules(_builtins_pkg.__path__): importlib.import_module(f"{_builtins_pkg.__name__}.{mod_info.name}") seen: set[type[Tool]] = set() for cls in Tool.__subclasses__(): stack: list[type[Tool]] = [cls] while stack: node = stack.pop() if node in seen: continue seen.add(node) stack.extend(node.__subclasses__()) return [ c for c in seen if c.__module__.startswith(_builtins_pkg.__name__) and not c.__name__.startswith("_") ] def test_async_builtins_override_dispatch_async_or_are_runner_dispatched() -> None: """``is_async()==True`` tools either override ``dispatch_async`` OR are runner-dispatched. The base ``Tool.dispatch_async`` raises ``NotImplementedError``, so any tool that flips ``is_async`` true without an override would crash the in-process Omnigent loop. After the DBOS removal, a class of async-namespace tools (``sys_call_async``, ``sys_read_inbox``, ``sys_cancel_async``) are dispatched by the runner via ``omnigent/runner/tool_dispatch.py`` — ``dispatch_async`` is never reached on those, so leaving them on the base implementation is correct. Pin the contract: an async tool is permitted iff it either overrides ``dispatch_async`` or is listed in the runner's ``_ALL_LOCAL_TOOLS`` set. """ from omnigent.runner.tool_dispatch import should_dispatch_locally base_dispatch_async = Tool.dispatch_async offenders = [] for cls in _all_builtin_tool_subclasses(): try: instance = cls() except Exception: # Tools that require constructor args are exempt — they # can't be instantiated bare here; their own tests cover # the async contract. continue if not instance.is_async(): continue if cls.dispatch_async is not base_dispatch_async: continue if should_dispatch_locally(cls.name()): continue offenders.append(f"{cls.__module__}.{cls.__name__}") assert not offenders, ( f"is_async()==True without dispatch_async override AND not " f"runner-dispatched (would raise NotImplementedError in the " f"in-process loop): {offenders}" )