From d105376b21d15a7c91f1a60c9bf0d0012ec2e841 Mon Sep 17 00:00:00 2001 From: Siddharth Balyan <52913345+alt-glitch@users.noreply.github.com> Date: Tue, 15 Sep 2026 00:41:13 +0530 Subject: [PATCH] Connector code lives in one package, tools/connectors/ (move only; NS-868 prep) (#110368) * refactor(tools): discovery also scans tools//tool.py A tool family that is a whole package had no way to register: discovery globbed tools/*.py only and derived the module name from the filename. Now the candidate list is tools/*.py plus tools/*/tool.py, merged and sorted once so import order does not depend on depth (register() lets a same-name duplicate overwrite silently), and the module name comes from the path relative to tools/. Only tool.py is scanned inside a package, so its siblings are libraries by construction. A package without an __init__.py is skipped with a warning rather than registering from a checkout and vanishing from the installed wheel. The AST prefilter and the (mtime, size) disk cache are per absolute path and work unchanged. The two hand-rolled tools/*.py enumerators in tests now use the same candidate helper. * refactor(connectors): one package for the connector domain, tools/connectors/ The connector code was spread across six flat files and a root-level module that was a sibling of model_tools.py only by address: tools/connections_tool.py -> tools/connectors/tool.py (schema, register, dispatcher) tools/connectors/managed.py (the managed leg, split out) tools/connections_tool_mcp.py -> tools/connectors/mcp.py (validation split out ->) tools/connectors/targets.py (normalize_targets, validate_action) tools/connections_tool_operation.py -> tools/connectors/operation.py tools/connector_search.py -> tools/connectors/search.py model_tools_connectors.py -> tools/connectors/dispatch.py tools/tool_gateway/ -> tools/connectors/gateway/ Move only; every function body is unchanged. tools/connectors/__init__.py is the door: nine names, the whole cross-package surface. model_tools and tool_search deep-import a few helpers past it on purpose and the docstring says so. The two split files make the import graph one-directional (tool -> mcp -> targets, tool -> managed) where the old layout had connections_tool importing validation out of the MCP file. One behaviour-neutral seam change: the _connectors_available try/except wrapper is gone. connectors_available() already fails closed, and both the registry handler and the inline executor now read it as a module attribute (gateway.config.connectors_available), so tests patch it in one place instead of two. _default_client lives in managed.py, the only module that calls it. tools/managed_tool_gateway.py and tools/managed_gateway_auth.py stay: they are gateway identity shared by tts, transcription, image and modal. Test files follow their modules. No docs referenced the old paths; no compat pointer is added (in-tree moves get none). * ci: retrigger (zero-job dispatch on 142466de6b) --- agent/inline_tool_executors.py | 5 +- model_tools.py | 9 +- tests/agent/test_run_agent.py | 4 +- tests/tools/test_connector_bridge_wiring.py | 8 +- tests/tools/test_connector_dispatch_policy.py | 11 +- tests/tools/test_connector_local_batches.py | 2 +- tests/tools/test_connector_names.py | 2 +- tests/tools/test_connector_session_scope.py | 6 +- ...t.py => test_connectors_gateway_client.py} | 12 +- ...ge.py => test_connectors_gateway_merge.py} | 10 +- ...ons_tool_mcp.py => test_connectors_mcp.py} | 12 +- ...ration.py => test_connectors_operation.py} | 2 +- ...ctions_tool.py => test_connectors_tool.py} | 8 +- tests/tools/test_registry.py | 63 +++++- tests/tui_gateway/contracts/test_generated.py | 4 +- tools/AGENTS.md | 5 +- tools/connectors/__init__.py | 41 ++++ .../connectors/dispatch.py | 6 +- .../gateway}/__init__.py | 8 +- .../gateway}/bridge.py | 12 +- .../gateway}/client.py | 6 +- .../gateway}/config.py | 0 .../gateway}/errors.py | 0 .../gateway}/merge.py | 4 +- .../gateway}/names.py | 0 .../gateway}/wire.py | 0 .../managed.py} | 181 ++---------------- .../mcp.py} | 74 +------ .../operation.py} | 0 .../search.py} | 6 +- tools/connectors/targets.py | 78 ++++++++ tools/connectors/tool.py | 175 +++++++++++++++++ tools/registry.py | 21 +- tools/tool_search.py | 4 +- tui_gateway/methods_connectors.py | 2 +- tui_gateway/tool_progress.py | 2 +- 36 files changed, 471 insertions(+), 312 deletions(-) rename tests/tools/{test_tool_gateway_client.py => test_connectors_gateway_client.py} (96%) rename tests/tools/{test_tool_gateway_merge.py => test_connectors_gateway_merge.py} (97%) rename tests/tools/{test_connections_tool_mcp.py => test_connectors_mcp.py} (95%) rename tests/tools/{test_connections_tool_operation.py => test_connectors_operation.py} (98%) rename tests/tools/{test_connections_tool.py => test_connectors_tool.py} (98%) create mode 100644 tools/connectors/__init__.py rename model_tools_connectors.py => tools/connectors/dispatch.py (93%) rename tools/{tool_gateway => connectors/gateway}/__init__.py (92%) rename tools/{tool_gateway => connectors/gateway}/bridge.py (94%) rename tools/{tool_gateway => connectors/gateway}/client.py (98%) rename tools/{tool_gateway => connectors/gateway}/config.py (100%) rename tools/{tool_gateway => connectors/gateway}/errors.py (100%) rename tools/{tool_gateway => connectors/gateway}/merge.py (98%) rename tools/{tool_gateway => connectors/gateway}/names.py (100%) rename tools/{tool_gateway => connectors/gateway}/wire.py (100%) rename tools/{connections_tool.py => connectors/managed.py} (67%) rename tools/{connections_tool_mcp.py => connectors/mcp.py} (62%) rename tools/{connections_tool_operation.py => connectors/operation.py} (100%) rename tools/{connector_search.py => connectors/search.py} (95%) create mode 100644 tools/connectors/targets.py create mode 100644 tools/connectors/tool.py diff --git a/agent/inline_tool_executors.py b/agent/inline_tool_executors.py index bcb6669bbc..3e15f8a0b7 100644 --- a/agent/inline_tool_executors.py +++ b/agent/inline_tool_executors.py @@ -152,12 +152,13 @@ def _desktop_preview(agent, args: dict, ctx: InlineToolContext) -> Any: def _manage_connections(agent, args: dict, ctx: InlineToolContext) -> Any: # The GUI callback lives on the agent; registry dispatch never forwards it. - from tools.connections_tool import _connectors_available, manage_connections + from tools.connectors import manage_connections + from tools.connectors.gateway import config as gateway_config return manage_connections( args, session_id=getattr(agent, "session_id", None), connection_callback=getattr(agent, "connection_callback", None), - connectors_available=_connectors_available, + connectors_available=gateway_config.connectors_available, ) diff --git a/model_tools.py b/model_tools.py index 4193fb2210..924cd94413 100644 --- a/model_tools.py +++ b/model_tools.py @@ -817,9 +817,8 @@ def _execute_tool(function_name: str, function_args: Dict[str, Any], original_ar dispatch_kwargs["user_task"] = user_task def _dispatch(next_args: Dict[str, Any]) -> Any: - from tools.tool_gateway.names import is_connector_name + from tools.connectors import dispatch_connector_call, is_connector_name if is_connector_name(function_name): - from model_tools_connectors import dispatch_connector_call return dispatch_connector_call(function_name, next_args, ids.tool_call_id) return registry.dispatch(function_name, next_args, **dispatch_kwargs) @@ -893,9 +892,8 @@ def handle_function_call( result, underlying = bridged if underlying is None: return _emit(result, duration_ms=_elapsed_ms(start)) - from tools.tool_gateway.names import CONNECTOR_BATCH_SENTINEL + from tools.connectors import CONNECTOR_BATCH_SENTINEL, dispatch_connector_batch if underlying[0] == CONNECTOR_BATCH_SENTINEL: - from model_tools_connectors import dispatch_connector_batch return _emit(dispatch_connector_batch( underlying[1]["calls"], ids, user_task=user_task, enabled_tools=enabled_tools, middleware_trace=trace, @@ -908,7 +906,8 @@ def handle_function_call( enabled_toolsets=enabled_toolsets, disabled_toolsets=disabled_toolsets, ) - from tools.tool_gateway.names import is_connector_name, parse_connector_name + from tools.connectors import is_connector_name + from tools.connectors.gateway.names import parse_connector_name if function_name == "manage_connections" or is_connector_name(function_name): if "manage_connections" not in _select_tool_names(enabled_toolsets, disabled_toolsets, quiet_mode=True): return _emit(tool_error("Connectors are not available in this session.")) diff --git a/tests/agent/test_run_agent.py b/tests/agent/test_run_agent.py index f2252cd39c..e85a1c1c05 100644 --- a/tests/agent/test_run_agent.py +++ b/tests/agent/test_run_agent.py @@ -2549,8 +2549,8 @@ class TestAgentRuntimePostHookOwnershipSync: ) # manage_connections / setup_mcp shim: no GUI callback on this fake agent, so the MCP # leg settles `unavailable` without a card; pin the catalog so the run is hermetic. - monkeypatch.setattr("tools.connections_tool_mcp._catalog_names", lambda: ["linear"]) - monkeypatch.setattr("tools.connections_tool_mcp._configured_names", lambda: []) + monkeypatch.setattr("tools.connectors.mcp._catalog_names", lambda: ["linear"]) + monkeypatch.setattr("tools.connectors.mcp._configured_names", lambda: []) monkeypatch.setattr(agent, "_get_session_db_for_recall", lambda: None) monkeypatch.setattr( agent, diff --git a/tests/tools/test_connector_bridge_wiring.py b/tests/tools/test_connector_bridge_wiring.py index 214019d374..4ac1d7a0f9 100644 --- a/tests/tools/test_connector_bridge_wiring.py +++ b/tests/tools/test_connector_bridge_wiring.py @@ -11,7 +11,7 @@ import logging import pytest from agent.tool_dispatch_helpers import _peel_bridge_call -from tools.tool_gateway.bridge import connector_describe +from tools.connectors.gateway.bridge import connector_describe from tools.tool_search import ( CONNECTOR_BATCH_SENTINEL, ToolSearchConfig, @@ -278,7 +278,7 @@ def test_search_keeps_only_the_twin_a_colliding_name_reaches(order, caplog): }, } - with caplog.at_level(logging.WARNING, logger="tools.connector_search"): + with caplog.at_level(logging.WARNING, logger="tools.connectors.search"): out = json.loads(dispatch_tool_search( {"queries": ["gmail fetch profile"]}, current_tool_defs=_local_defs(), @@ -459,7 +459,7 @@ def test_peel_keeps_mixed_and_local_batches_as_sequential_barrier(): def _connectors_on(monkeypatch, client_factory): from tools.registry import invalidate_check_fn_cache - from tools.tool_gateway import bridge, config + from tools.connectors.gateway import bridge, config monkeypatch.setattr(config, "connectors_available", lambda: True) monkeypatch.setattr(bridge, "connectors_available", lambda: True) @@ -568,7 +568,7 @@ class _RecordingTransport: def _recording_client_factory(transport): - from tools.tool_gateway.client import ConnectorClient + from tools.connectors.gateway.client import ConnectorClient return lambda: ConnectorClient( transport=transport, diff --git a/tests/tools/test_connector_dispatch_policy.py b/tests/tools/test_connector_dispatch_policy.py index 523dfcab00..cf105fb69f 100644 --- a/tests/tools/test_connector_dispatch_policy.py +++ b/tests/tools/test_connector_dispatch_policy.py @@ -10,7 +10,7 @@ def test_remote_entries_run_request_hook_and_execution_policies(monkeypatch, blo import model_tools import hermes_cli.plugins as plugins from tools.registry import invalidate_check_fn_cache - from tools.tool_gateway import bridge, config + from tools.connectors.gateway import bridge, config monkeypatch.setattr(config, "connectors_available", lambda: True) monkeypatch.setattr(bridge, "connectors_available", lambda: True) @@ -81,7 +81,7 @@ def test_stop_during_a_connector_batch_leaves_unstarted_entries_unsent(monkeypat import model_tools from tools.interrupt import set_interrupt from tools.registry import invalidate_check_fn_cache - from tools.tool_gateway import bridge, config + from tools.connectors.gateway import bridge, config monkeypatch.setattr(config, "connectors_available", lambda: True) monkeypatch.setattr(bridge, "connectors_available", lambda: True) @@ -112,11 +112,12 @@ def test_stop_during_a_connector_batch_leaves_unstarted_entries_unsent(monkeypat def test_disabled_connections_cannot_be_called_through_a_stale_schema(monkeypatch): - from tools import connections_tool + from tools.connectors import managed + from tools.connectors.gateway import config from tools.registry import registry - monkeypatch.setattr(connections_tool, "_connectors_available", lambda: False) - monkeypatch.setattr(connections_tool, "_default_client", + monkeypatch.setattr(config, "connectors_available", lambda: False) + monkeypatch.setattr(managed, "_default_client", lambda: (_ for _ in ()).throw(AssertionError("disabled connector attempted I/O"))) result = json.loads(registry.dispatch("manage_connections", {"action": "connect", "connectors": ["gmail"]})) assert "not available" in result["error"] diff --git a/tests/tools/test_connector_local_batches.py b/tests/tools/test_connector_local_batches.py index a422f06c8b..8a6bc0cb9f 100644 --- a/tests/tools/test_connector_local_batches.py +++ b/tests/tools/test_connector_local_batches.py @@ -10,7 +10,7 @@ import pytest def test_local_batches_rejected_before_any_entry_executes(monkeypatch, mixed): import model_tools from tools.tool_search import resolve_underlying_call - from tools.tool_gateway import bridge, config + from tools.connectors.gateway import bridge, config from tools.registry import invalidate_check_fn_cache monkeypatch.setattr(config, "connectors_available", lambda: True) diff --git a/tests/tools/test_connector_names.py b/tests/tools/test_connector_names.py index 07e4c0ab03..0a03352a45 100644 --- a/tests/tools/test_connector_names.py +++ b/tests/tools/test_connector_names.py @@ -2,7 +2,7 @@ import pytest -from tools.tool_gateway.names import ( +from tools.connectors.gateway.names import ( format_connector_name, parse_connector_name, vendor_slug_candidates, diff --git a/tests/tools/test_connector_session_scope.py b/tests/tools/test_connector_session_scope.py index 40284e29c0..c9cf572a77 100644 --- a/tests/tools/test_connector_session_scope.py +++ b/tests/tools/test_connector_session_scope.py @@ -17,8 +17,8 @@ import pytest ]) def test_connector_scope_controls_schema_discovery_and_execution(monkeypatch, enabled, disabled, allowed): import model_tools - from tools.tool_gateway import bridge, config - from tools import connections_tool + from tools.connectors import managed + from tools.connectors.gateway import bridge, config monkeypatch.setattr(config, "connectors_available", lambda: True) monkeypatch.setattr(bridge, "connectors_available", lambda: True) @@ -46,7 +46,7 @@ def test_connector_scope_controls_schema_discovery_and_execution(monkeypatch, en return [] monkeypatch.setattr(bridge, "_default_client_factory", Client) - monkeypatch.setattr(connections_tool, "_default_client", Client) + monkeypatch.setattr(managed, "_default_client", Client) scope = {"enabled_toolsets": enabled, "disabled_toolsets": disabled} defs = model_tools.get_tool_definitions(**scope, quiet_mode=True, skip_tool_search_assembly=True) assert ("manage_connections" in {td["function"]["name"] for td in defs}) is allowed diff --git a/tests/tools/test_tool_gateway_client.py b/tests/tools/test_connectors_gateway_client.py similarity index 96% rename from tests/tools/test_tool_gateway_client.py rename to tests/tools/test_connectors_gateway_client.py index 5c84f8810e..f09cb8b6c4 100644 --- a/tests/tools/test_tool_gateway_client.py +++ b/tests/tools/test_connectors_gateway_client.py @@ -10,15 +10,15 @@ from dataclasses import replace as dataclass_replace import pytest -from tools.tool_gateway.bridge import connector_search_hits -from tools.tool_gateway.client import ConnectorClient -from tools.tool_gateway.errors import ( +from tools.connectors.gateway.bridge import connector_search_hits +from tools.connectors.gateway.client import ConnectorClient +from tools.connectors.gateway.errors import ( GatewayAuthError, GatewayUnavailable, IdempotencyConflict, ToolGatewayError, ) -from tools.tool_gateway.names import vendor_slug_candidates +from tools.connectors.gateway.names import vendor_slug_candidates class FakeResponse: @@ -73,7 +73,7 @@ PLAN_CALLS = [ def planned(calls=PLAN_CALLS): - from tools.tool_gateway.merge import partition_calls + from tools.connectors.gateway.merge import partition_calls return tuple( dataclass_replace( @@ -297,7 +297,7 @@ def _resolve_with_env(**overrides): import os from unittest.mock import patch - from tools.tool_gateway.client import _default_endpoint_resolver + from tools.connectors.gateway.client import _default_endpoint_resolver env = {k: v for k, v in os.environ.items() if k not in _GATEWAY_ENV_KEYS} env.update(overrides) diff --git a/tests/tools/test_tool_gateway_merge.py b/tests/tools/test_connectors_gateway_merge.py similarity index 97% rename from tests/tools/test_tool_gateway_merge.py rename to tests/tools/test_connectors_gateway_merge.py index a2fe19c2ea..d9b82df07e 100644 --- a/tests/tools/test_tool_gateway_merge.py +++ b/tests/tools/test_connectors_gateway_merge.py @@ -1,4 +1,4 @@ -"""Behavior tests for the pure tool_gateway merge/partition/name logic. +"""Behavior tests for the pure connectors.gateway merge/partition/name logic. Pure functions, zero fakes, no I/O — matching the DI-callable test idiom (``test_managed_tool_gateway.py``). Wire/client behavior is covered in the @@ -7,21 +7,21 @@ client PR; this file owns partition → splice → assemble and the name codec. import pytest -from tools.tool_gateway.config import ConnectorConfig, connectors_available -from tools.tool_gateway.errors import ( +from tools.connectors.gateway.config import ConnectorConfig, connectors_available +from tools.connectors.gateway.errors import ( GatewayAuthError, GatewayUnavailable, IdempotencyConflict, ToolGatewayError, parse_gateway_error, ) -from tools.tool_gateway.merge import ( +from tools.connectors.gateway.merge import ( assemble_results, fill_remote_failure, partition_calls, splice_remote_results, ) -from tools.tool_gateway.names import ( +from tools.connectors.gateway.names import ( CONNECTOR_BATCH_SENTINEL, format_connector_name, parse_connector_name, diff --git a/tests/tools/test_connections_tool_mcp.py b/tests/tools/test_connectors_mcp.py similarity index 95% rename from tests/tools/test_connections_tool_mcp.py rename to tests/tools/test_connectors_mcp.py index fd756f7966..11deab254e 100644 --- a/tests/tools/test_connections_tool_mcp.py +++ b/tests/tools/test_connectors_mcp.py @@ -16,9 +16,9 @@ from unittest.mock import patch import pytest -import tools.connections_tool # registers the tool -from tools import connections_tool_operation as op -from tools.connections_tool import MANAGE_CONNECTIONS_SCHEMA, manage_connections +import tools.connectors.tool # registers the tool +from tools.connectors import operation as op +from tools.connectors.tool import MANAGE_CONNECTIONS_SCHEMA, manage_connections from tools.registry import registry CATALOG = ["figma", "linear", "notion"] @@ -27,8 +27,8 @@ CONFIGURED = {"paper": {"command": "paper-mcp"}, "linear": {"url": "https://mcp. @pytest.fixture(autouse=True) def _catalog(): - with patch("tools.connections_tool_mcp._catalog_names", return_value=CATALOG), \ - patch("tools.connections_tool_mcp._configured_names", return_value=sorted(CONFIGURED)): + with patch("tools.connectors.mcp._catalog_names", return_value=CATALOG), \ + patch("tools.connectors.mcp._configured_names", return_value=sorted(CONFIGURED)): yield @@ -220,7 +220,7 @@ def test_the_bounded_wait_owns_the_deadline_not_the_sequential_guard(): def test_default_wait_comes_from_the_config_key(monkeypatch): monkeypatch.setenv("HERMES_CONCURRENT_TOOL_TIMEOUT_S", "3") seen = {} - with patch("tools.connections_tool_mcp.resolve_wait_timeout", return_value=77.0): + with patch("tools.connectors.mcp.resolve_wait_timeout", return_value=77.0): manage_connections({"action": "install", "connectors": [_linear()]}, connection_callback=lambda p: seen.update(p) or "") assert seen["timeout_seconds"] == 77.0 diff --git a/tests/tools/test_connections_tool_operation.py b/tests/tools/test_connectors_operation.py similarity index 98% rename from tests/tools/test_connections_tool_operation.py rename to tests/tools/test_connectors_operation.py index aecb5a2391..629c347d01 100644 --- a/tests/tools/test_connections_tool_operation.py +++ b/tests/tools/test_connectors_operation.py @@ -2,7 +2,7 @@ import pytest -from tools import connections_tool_operation as op +from tools.connectors import operation as op def _two_targets(): diff --git a/tests/tools/test_connections_tool.py b/tests/tools/test_connectors_tool.py similarity index 98% rename from tests/tools/test_connections_tool.py rename to tests/tools/test_connectors_tool.py index 68b246e707..a8269d3a36 100644 --- a/tests/tools/test_connections_tool.py +++ b/tests/tools/test_connectors_tool.py @@ -10,8 +10,8 @@ from unittest.mock import patch import pytest -import tools.connections_tool # registers the tool -from tools.connections_tool import MANAGE_CONNECTIONS_SCHEMA, manage_connections +import tools.connectors.tool # registers the tool +from tools.connectors.tool import MANAGE_CONNECTIONS_SCHEMA, manage_connections class FakeClient: @@ -434,7 +434,7 @@ def _session_tool_names(enabled_toolsets, *, connectors, disabled_toolsets=None) from model_tools import _compute_tool_definitions from tools.registry import invalidate_check_fn_cache - with patch("tools.tool_gateway.config.connectors_available", + with patch("tools.connectors.gateway.config.connectors_available", return_value=connectors): invalidate_check_fn_cache() try: @@ -515,7 +515,7 @@ def test_signed_out_session_keeps_the_tool_but_the_managed_leg_refuses(tmp_path, for selection in selections: assert "manage_connections" in _session_tool_names(selection, connectors=False), selection - with patch("tools.connections_tool._connectors_available", return_value=False): + with patch("tools.connectors.gateway.config.connectors_available", return_value=False): out = json.loads(registry.dispatch("manage_connections", {"action": "status"})) assert "not available in this session" in out["error"] diff --git a/tests/tools/test_registry.py b/tests/tools/test_registry.py index 8b964dd949..9390b34414 100644 --- a/tests/tools/test_registry.py +++ b/tests/tools/test_registry.py @@ -13,6 +13,7 @@ from tools.registry import ( _MAX_LOGGED_ERROR_CHARS, _MAX_TOOL_ERROR_CHARS, _module_registers_tools, + _tool_module_candidates, discover_builtin_tools, tool_error, ) @@ -353,8 +354,8 @@ class TestBuiltinDiscovery: def test_discovers_all_real_self_registering_builtin_tool_modules(self): tools_dir = Path(__file__).resolve().parents[2] / "tools" expected = [ - f"tools.{path.stem}" - for path in sorted(tools_dir.glob("*.py")) + ".".join(("tools", *path.relative_to(tools_dir).with_suffix("").parts)) + for path in _tool_module_candidates(tools_dir) if path.name not in {"__init__.py", "registry.py", "mcp_tool.py"} and _module_registers_tools(path) ] @@ -385,6 +386,64 @@ class TestBuiltinDiscovery: mock_import.assert_called_once_with("tools.alpha") +_REGISTERING_SOURCE = ( + "from tools.registry import registry\n" + "registry.register(name='conn', toolset='x', schema={}, handler=lambda *_a, **_k: '{}')\n" +) + + +class TestPackageToolDiscovery: + """A package under tools/ registers its model tool from ``/tool.py`` and nothing else.""" + + @staticmethod + def _make_pkg(tmp_path, *, init=True): + tools_dir = tmp_path / "tools" + pkg = tools_dir / "connectors" + pkg.mkdir(parents=True) + (tools_dir / "__init__.py").write_text("", encoding="utf-8") + if init: + (pkg / "__init__.py").write_text("", encoding="utf-8") + (pkg / "tool.py").write_text(_REGISTERING_SOURCE, encoding="utf-8") + return tools_dir, pkg + + def test_package_tool_py_is_imported_under_its_dotted_name(self, tmp_path): + tools_dir, _ = self._make_pkg(tmp_path) + with patch("tools.registry.importlib.import_module") as mock_import: + imported = discover_builtin_tools(tools_dir) + assert imported == ["tools.connectors.tool"] + mock_import.assert_called_once_with("tools.connectors.tool") + + def test_package_siblings_are_libraries_not_scanned(self, tmp_path): + tools_dir, pkg = self._make_pkg(tmp_path) + # Registers at module level, but is not the package's tool.py: discovery must not import it. + (pkg / "operation.py").write_text(_REGISTERING_SOURCE, encoding="utf-8") + with patch("tools.registry.importlib.import_module") as mock_import: + imported = discover_builtin_tools(tools_dir) + assert imported == ["tools.connectors.tool"] + assert {c.args[0] for c in mock_import.call_args_list} == {"tools.connectors.tool"} + + def test_package_without_init_is_skipped_loudly(self, tmp_path, caplog): + tools_dir, _ = self._make_pkg(tmp_path, init=False) + with patch("tools.registry.importlib.import_module") as mock_import, caplog.at_level( + logging.WARNING, logger="tools.registry" + ): + imported = discover_builtin_tools(tools_dir) + assert imported == [] + mock_import.assert_not_called() + assert any("__init__.py" in rec.getMessage() for rec in caplog.records) + + def test_cache_round_trips_the_nested_verdict(self, tmp_path): + tools_dir, _ = self._make_pkg(tmp_path) + with patch("tools.registry.importlib.import_module"): + discover_builtin_tools(tools_dir) + with patch( + "tools.registry._module_registers_tools", + side_effect=AssertionError("nested file was re-scanned despite a cache hit"), + ): + imported = discover_builtin_tools(tools_dir) + assert imported == ["tools.connectors.tool"] + + class TestEmojiMetadata: """Verify per-tool emoji registration and lookup.""" diff --git a/tests/tui_gateway/contracts/test_generated.py b/tests/tui_gateway/contracts/test_generated.py index 11a8638a3b..b74d813960 100644 --- a/tests/tui_gateway/contracts/test_generated.py +++ b/tests/tui_gateway/contracts/test_generated.py @@ -66,7 +66,9 @@ def emitted_event_names() -> set[str]: for src in (REPO / "tools").glob("delegate_tool*.py"): names.update(_SUBAGENT_RELAY.findall(_read(src))) names.discard("subagent.text") # mirrored into the watch window as message.delta, never emitted - for src in (REPO / "tools").glob("*.py"): + from tools.registry import _tool_module_candidates + + for src in _tool_module_candidates(REPO / "tools"): names.update(_DESKTOP_UI_EMIT.findall(_read(src))) names.update(_BROKER_FRAME.findall(_read(REPO / "gateway" / "browser_control_broker.py"))) names.update(_SETUP_READY.findall(_read(REPO / "hermes_cli" / "free_tier_bootstrap.py"))) diff --git a/tools/AGENTS.md b/tools/AGENTS.md index e6af2fd704..41c7775f6d 100644 --- a/tools/AGENTS.md +++ b/tools/AGENTS.md @@ -10,7 +10,10 @@ Most capabilities should NOT be core tools. Long-form: `website/docs/developer-g `registry.register()` at import time; `model_tools.py` imports the registry and triggers discovery (`discover_builtin_tools()`), then `run_agent.py`, `cli.py`, `batch_runner.py`, `environments/` consume it. Any `tools/*.py` with a top-level `registry.register()` is imported automatically — no -manual import list. The registry handles schema collection, dispatch (`handle_function_call()`), +manual import list. A tool that is a whole package (`tools/connectors/`) registers from +`tools//tool.py`, the only file discovery scans inside a package; every sibling in the package +is a library by construction, and the package needs an `__init__.py` or discovery skips it with a +warning (setuptools would drop it from the wheel). The registry handles schema collection, dispatch (`handle_function_call()`), availability (`check_fn`, TTL-cached process-wide), and error wrapping. **All handlers return a JSON string.** diff --git a/tools/connectors/__init__.py b/tools/connectors/__init__.py new file mode 100644 index 0000000000..2e503421d0 --- /dev/null +++ b/tools/connectors/__init__.py @@ -0,0 +1,41 @@ +"""Connectors: managed connector accounts (Nous tool gateway) and local MCP servers behind one +model tool, ``manage_connections``. + +Layout (a deep module with a narrow door): + +- ``tool.py`` — the model tool: schema, ``registry.register`` (the one file discovery scans in this + package), and the action dispatcher. +- ``targets.py`` — target and action validation shared by every leg. Pure. +- ``operation.py`` — ``ConnectionOperation`` / ``Target``: targets, server-owned deadline, + exactly-once settlement. Pure data, no I/O. +- ``mcp.py`` — the local-MCP leg (install / enable / authorize through the approval card). +- ``managed.py`` — the managed-connector leg (status / connect / reconnect / wait). +- ``search.py`` — the ``tool_search`` / ``tool_describe`` adapter for remote connector tools. +- ``dispatch.py`` — remote connector calls re-entering ``model_tools.handle_function_call`` so + per-tool policy runs against the composed ``connectors____`` name. +- ``gateway/`` — the typed client for the tool gateway's connector routes; see its own docstring + for the layering rules inside it. + +The names below are the whole cross-package surface. ``model_tools`` and ``tool_search`` are +core plumbing and deep-import a few helpers past this door on purpose; nothing else should. An +importer outside this package needing a name not listed here is a design question, not a reason +to add the name. +""" + +from tools.connectors.dispatch import dispatch_connector_batch, dispatch_connector_call +from tools.connectors.gateway.bridge import connector_describe, connector_search_hits +from tools.connectors.gateway.config import connectors_available +from tools.connectors.gateway.names import CONNECTOR_BATCH_SENTINEL, is_connector_name +from tools.connectors.tool import MANAGE_CONNECTIONS_SCHEMA, manage_connections + +__all__ = [ + "CONNECTOR_BATCH_SENTINEL", + "MANAGE_CONNECTIONS_SCHEMA", + "connector_describe", + "connector_search_hits", + "connectors_available", + "dispatch_connector_batch", + "dispatch_connector_call", + "is_connector_name", + "manage_connections", +] diff --git a/model_tools_connectors.py b/tools/connectors/dispatch.py similarity index 93% rename from model_tools_connectors.py rename to tools/connectors/dispatch.py index 915a264fb8..b963f82e40 100644 --- a/model_tools_connectors.py +++ b/tools/connectors/dispatch.py @@ -4,8 +4,8 @@ import json from dataclasses import asdict from tools.registry import tool_error -from tools.tool_gateway.config import MAX_CALLS_PER_DISPATCH -from tools.tool_gateway.merge import assemble_results, fill_remote_failure, partition_calls +from tools.connectors.gateway.config import MAX_CALLS_PER_DISPATCH +from tools.connectors.gateway.merge import assemble_results, fill_remote_failure, partition_calls def dispatch_connector_call(name, arguments, tool_call_id): @@ -14,7 +14,7 @@ def dispatch_connector_call(name, arguments, tool_call_id): Execution middleware wraps the actual I/O, so connector entries execute individually rather than queuing side effects after a policy callback returns. """ - from tools.tool_gateway.bridge import run_remote + from tools.connectors.gateway.bridge import run_remote partition = partition_calls([{"name": name, "arguments": arguments}]) entries = run_remote(partition.remote, tool_call_id, availability=None, client_factory=None) diff --git a/tools/tool_gateway/__init__.py b/tools/connectors/gateway/__init__.py similarity index 92% rename from tools/tool_gateway/__init__.py rename to tools/connectors/gateway/__init__.py index d0d59b7a2e..42c3c7af3d 100644 --- a/tools/tool_gateway/__init__.py +++ b/tools/connectors/gateway/__init__.py @@ -26,18 +26,18 @@ Layering rules (enforced by review, not imports — keep them true): Approval is settled by the core BEFORE the bridge is called; denied entries never reach it. -Core reaches this package through ``model_tools_connectors.py``, which +Core reaches this package through ``tools.connectors.dispatch.py``, which dispatches one gateway request per connector entry via ``bridge.run_remote`` and re-enters core dispatch for each entry so per-tool policy fires against the composed ``connectors__`` name. """ -from tools.tool_gateway.config import ( +from tools.connectors.gateway.config import ( MAX_CALLS_PER_DISPATCH, ConnectorConfig, connectors_available, ) -from tools.tool_gateway.errors import ( +from tools.connectors.gateway.errors import ( GatewayAuthError, GatewayUnavailable, IdempotencyConflict, @@ -45,7 +45,7 @@ from tools.tool_gateway.errors import ( parse_gateway_error, render_connection_required, ) -from tools.tool_gateway.names import ( +from tools.connectors.gateway.names import ( CONNECTOR_BATCH_SENTINEL, CONNECTOR_NAME_PREFIX, ConnectorName, diff --git a/tools/tool_gateway/bridge.py b/tools/connectors/gateway/bridge.py similarity index 94% rename from tools/tool_gateway/bridge.py rename to tools/connectors/gateway/bridge.py index e757bbd185..edf8ef32f4 100644 --- a/tools/tool_gateway/bridge.py +++ b/tools/connectors/gateway/bridge.py @@ -11,7 +11,7 @@ search behaving exactly as it does today. Transport leg: :func:`run_remote` sends one gateway execute request for the planned entries it is handed and splices the results back by slot. -``model_tools_connectors`` calls it once per connector entry, after core +``tools.connectors.dispatch`` calls it once per connector entry, after core dispatch has already run scope, hook, approval and middleware policy against that entry's composed ``connectors__`` name. Vendor slug restoration and the single literal-slug retry live here; partition and envelope assembly live in @@ -24,10 +24,10 @@ import logging from dataclasses import replace as dataclass_replace from typing import Any, Callable, Optional, Sequence -from tools.tool_gateway.config import connectors_available -from tools.tool_gateway.errors import GatewayUnavailable, ToolGatewayError -from tools.tool_gateway.merge import fill_remote_failure, splice_remote_results -from tools.tool_gateway.names import parse_connector_name, vendor_slug_candidates +from tools.connectors.gateway.config import connectors_available +from tools.connectors.gateway.errors import GatewayUnavailable, ToolGatewayError +from tools.connectors.gateway.merge import fill_remote_failure, splice_remote_results +from tools.connectors.gateway.names import parse_connector_name, vendor_slug_candidates logger = logging.getLogger(__name__) @@ -35,7 +35,7 @@ __all__ = ["connector_describe", "connector_search_hits", "run_remote"] def _default_client_factory(): - from tools.tool_gateway.client import ConnectorClient + from tools.connectors.gateway.client import ConnectorClient return ConnectorClient() diff --git a/tools/tool_gateway/client.py b/tools/connectors/gateway/client.py similarity index 98% rename from tools/tool_gateway/client.py rename to tools/connectors/gateway/client.py index 01a260afb5..40dbe34540 100644 --- a/tools/tool_gateway/client.py +++ b/tools/connectors/gateway/client.py @@ -34,14 +34,14 @@ from typing import Any, Callable, Optional, Protocol, Sequence import requests -from tools.tool_gateway import wire -from tools.tool_gateway.errors import ( +from tools.connectors.gateway import wire +from tools.connectors.gateway.errors import ( GatewayAuthError, GatewayUnavailable, ToolGatewayError, parse_gateway_error, ) -from tools.tool_gateway.merge import PlannedCall +from tools.connectors.gateway.merge import PlannedCall logger = logging.getLogger(__name__) diff --git a/tools/tool_gateway/config.py b/tools/connectors/gateway/config.py similarity index 100% rename from tools/tool_gateway/config.py rename to tools/connectors/gateway/config.py diff --git a/tools/tool_gateway/errors.py b/tools/connectors/gateway/errors.py similarity index 100% rename from tools/tool_gateway/errors.py rename to tools/connectors/gateway/errors.py diff --git a/tools/tool_gateway/merge.py b/tools/connectors/gateway/merge.py similarity index 98% rename from tools/tool_gateway/merge.py rename to tools/connectors/gateway/merge.py index 2b57eb7557..4ca82b46f1 100644 --- a/tools/tool_gateway/merge.py +++ b/tools/connectors/gateway/merge.py @@ -30,8 +30,8 @@ from __future__ import annotations from dataclasses import dataclass from typing import Any, Mapping, Optional, Sequence -from tools.tool_gateway.errors import render_connection_required -from tools.tool_gateway.names import parse_connector_name +from tools.connectors.gateway.errors import render_connection_required +from tools.connectors.gateway.names import parse_connector_name __all__ = [ "Partition", diff --git a/tools/tool_gateway/names.py b/tools/connectors/gateway/names.py similarity index 100% rename from tools/tool_gateway/names.py rename to tools/connectors/gateway/names.py diff --git a/tools/tool_gateway/wire.py b/tools/connectors/gateway/wire.py similarity index 100% rename from tools/tool_gateway/wire.py rename to tools/connectors/gateway/wire.py diff --git a/tools/connections_tool.py b/tools/connectors/managed.py similarity index 67% rename from tools/connections_tool.py rename to tools/connectors/managed.py index 8f0ed9f150..8996282fbc 100644 --- a/tools/connections_tool.py +++ b/tools/connectors/managed.py @@ -1,30 +1,12 @@ -#!/usr/bin/env python3 -"""Manage remote connector accounts served through the tool gateway. +"""Managed connectors (Nous tool gateway): status, connect / reconnect, and the in-call ``wait``. -``manage_connections`` is the never-deferred surface for connection -lifecycle: - -- ``status`` — which connectors exist for this account and whether each is - connected (read-only). -- ``connect`` / ``reconnect`` — start (or restart) an authorization flow. - The gateway returns a connect link, passed through UN-redacted: the model - shows it to the user, who opens it in a browser. Each connector's - ``instruction`` text is surfaced once per session, not on every call. -- ``wait`` — block inside the call until the named connectors report - connected, or the budget runs out. A model has no clock: told to wait it - says "I'll check back in a minute" and its next action lands immediately, - so guidance produced a burst of polls rather than a paced one. Waiting - inside the call cannot be skipped and works the same on every platform. - -Scope: managed connectors AND local MCP servers. ``{"name": ..., "mcp": true}`` targets take -``install`` / ``enable`` / ``authorize`` through one connection operation -(``connections_tool_operation.py``, ``connections_tool_mcp.py``); the approval card is reached -only via the inline executor, so non-GUI surfaces get ``unavailable`` for MCP targets. - -De-authentication is deliberately NOT exposed to the model: disconnecting -an account is a user decision, made in the portal dashboard. - -Availability: the managed leg is portal-gated in the handler; MCP targets need no sign-in. +``connect`` / ``reconnect`` start (or restart) an authorization flow. The gateway returns a connect +link, passed through UN-redacted: the model shows it to the user, who opens it in a browser. Each +connector's ``instruction`` text is surfaced once per session, not on every call. ``wait`` blocks +inside the call until the named connectors report connected, or the budget runs out. A model has +no clock: told to wait it says "I'll check back in a minute" and its next action lands +immediately, so guidance produced a burst of polls rather than a paced one. Waiting inside the +call cannot be skipped and works the same on every platform. """ import json @@ -33,19 +15,10 @@ import threading import time from typing import Any, Callable, Dict, List, Optional -from tools.connections_tool_mcp import ( - ALL_ACTIONS, - MCP_ACTIONS, - normalize_targets, - run_mcp_operation, - validate_action, -) -from tools.registry import registry, tool_error +from tools.registry import tool_error logger = logging.getLogger(__name__) -_CONNECTOR_ACTIONS = ("status", "connect", "reconnect", "wait") - # (session_id, connector) pairs whose `instruction` text has already been # shown. Keyed per session, not per process: the gateway multiplexes many # sessions through one process, and guidance suppressed for session A must @@ -106,18 +79,8 @@ _WAIT_UNFINISHED_NOTE = ( "broken." ) - -def _connectors_available() -> bool: - try: - from tools.tool_gateway.config import connectors_available - - return connectors_available() - except Exception: - return False - - def _default_client(): - from tools.tool_gateway.client import ConnectorClient + from tools.connectors.gateway.client import ConnectorClient return ConnectorClient() @@ -293,37 +256,21 @@ def _wait_for_connections( ) spent += gap - -def manage_connections( +def run_managed_action( + action: str, + connectors: List[str], args: Dict[str, Any], *, client_factory: Optional[Callable[[], Any]] = None, seen_instructions: Optional[set] = None, rendered_links: Optional[Dict[str, Dict[str, float]]] = None, session_id: Optional[str] = None, - connection_callback: Optional[Callable[[Dict[str, Any]], Optional[str]]] = None, connectors_available: Optional[Callable[[], bool]] = None, - wait_seconds: Optional[float] = None, ) -> str: - """Dispatch one ``manage_connections`` action. Returns a JSON string.""" - action = str(args.get("action") or "status").strip().lower() - managed, mcp_targets, target_error = normalize_targets(args.get("connectors")) - if target_error: - return tool_error(target_error) - action_error = validate_action(action, managed, mcp_targets) - if action_error: - return tool_error(action_error) - - if action in MCP_ACTIONS: - return run_mcp_operation( - mcp_targets, action, str(args.get("reason") or "").strip(), - connection_callback=connection_callback, session_id=session_id, wait_seconds=wait_seconds, - ) - - # Managed leg. Callers that pass a gate (registry handler, inline executor) are portal-gated. + """One managed-connector action (status / connect / reconnect / wait). Returns the tool's + JSON string. ``connectors`` are the normalized managed target names.""" if connectors_available is not None and not connectors_available(): return tool_error("Connectors are not available in this session.") - connectors: List[str] = managed try: client = (client_factory or _default_client)() @@ -441,101 +388,3 @@ def manage_connections( f"The connector gateway request failed: {exc}. " "If this persists, the user can manage connections in the Nous Portal." ) - - -MANAGE_CONNECTIONS_SCHEMA = { - "name": "manage_connections", - "description": ( - "Connect the user to apps: managed connector accounts (Gmail, Notion, ...) served " - "through the tool gateway, and local MCP servers from the catalog. Targets go in " - "'connectors': a bare slug or {\"name\": \"gmail\"} is a managed connector; " - "{\"name\": \"linear\", \"mcp\": true} is a local MCP server. " - "Managed actions: 'status' lists connectors and whether each is connected; 'connect' " - "starts an authorization for the given connectors and returns a link " - "for the USER to open in a browser (never open it yourself); " - "'reconnect' restarts a broken authorization; " - "'wait' blocks until the given connectors report connected. Pass " - "SEVERAL slugs in one call to get all authorization links at once. " - "When a connector tool " - "call returns CONNECTION_REQUIRED, use 'connect' and show the link. " - "Send the message that shows the user the links FIRST; on your NEXT " - "turn call 'wait' with those same slugs instead of guessing when the " - "user is done — it polls for you (a wait in the same turn as the " - "connect is bounced, because the user cannot have seen the links " - "yet). 'wait' requires 'connectors', and only accepts connectors this " - "session already addressed with 'connect' (already-connected apps " - "count). A 'timeout' or 'interrupted' result is NOT an " - "error: the user has not finished connecting, so ask them whether to " - "keep waiting, continue without those apps, or get fresh links. " - "MCP actions (targets must carry \"mcp\": true): 'install' adds a catalog entry, " - "'enable' re-enables a disabled configured server, 'authorize' runs its OAuth. " - "They show the user an approval card and block until it settles; the result lists " - "each target as connected / skipped / not_connected. Never hand-edit mcp_servers " - "config — always use this tool. Never re-ask after a skip or timeout: continue " - "without the server or ask in chat. A newly installed or authorized server's tools " - "arrive on your next turn. Off the desktop app the MCP targets come back " - "'unavailable' with the terminal commands to give the user. " - "This tool can NOT disconnect, delete, or revoke an account — that is " - "deliberately user-only. When asked, say so and direct the user to " - "the Nous Portal (their org's Connectors page) or the desktop app." - ), - "parameters": { - "type": "object", - "properties": { - "action": { - "type": "string", - "enum": list(ALL_ACTIONS), - "description": "Defaults to status. install/enable/authorize need mcp:true targets.", - }, - "connectors": { - "type": "array", - "items": { - "anyOf": [ - {"type": "string"}, - { - "type": "object", - "properties": { - "name": {"type": "string"}, - "mcp": {"type": "boolean", "description": "true = local MCP server."}, - }, - "required": ["name"], - "additionalProperties": False, - }, - ] - }, - "description": ( - "Targets. REQUIRED for every action but status " - "(e.g. [\"gmail\", {\"name\": \"linear\", \"mcp\": true}]); optional filter for status." - ), - }, - "reason": { - "type": "string", - "description": "MCP actions: one sentence on the approval card — why this helps right now.", - }, - "timeout_seconds": { - "type": "integer", - "description": ( - "For action 'wait' only: how long to hold the call open. " - f"Defaults to {int(_WAIT_DEFAULT_SECONDS)}, clamped to " - f"{int(_WAIT_MIN_SECONDS)}-{int(_WAIT_MAX_SECONDS)}. Ask for " - "more and the result carries a 'timeout_note' saying the cap " - "was applied; call wait again to keep waiting." - ), - }, - }, - "required": [], - }, -} - - -registry.register( - name="manage_connections", - toolset="connections", - schema=MANAGE_CONNECTIONS_SCHEMA, - # The portal gate is in the handler, not check_fn, so signed-out sessions keep the tool for - # MCP approvals. The registry path has no GUI callback. - handler=lambda args, **kw: manage_connections( - args, session_id=kw.get("session_id"), connectors_available=_connectors_available, - ), - emoji="🔗", -) diff --git a/tools/connections_tool_mcp.py b/tools/connectors/mcp.py similarity index 62% rename from tools/connections_tool_mcp.py rename to tools/connectors/mcp.py index 91e21cbca6..d4274259d5 100644 --- a/tools/connections_tool_mcp.py +++ b/tools/connectors/mcp.py @@ -1,14 +1,14 @@ -"""MCP targets of ``manage_connections``: target/action/catalog validation and the approval -leg. The card is reached via ``agent.connection_callback`` through the inline executor; -registry dispatch has no callback and settles targets ``unavailable``.""" +"""MCP targets of ``manage_connections``: catalog validation and the approval leg. The card is +reached via ``agent.connection_callback`` through the inline executor; registry dispatch has no +callback and settles targets ``unavailable``.""" from __future__ import annotations import json import logging -from typing import Any, Callable, Dict, List, Optional, Tuple +from typing import Any, Callable, Dict, List, Optional -from tools.connections_tool_operation import ( +from tools.connectors.operation import ( CONNECTED, FAILED, SETTLED_ALL_RESOLVED, @@ -26,12 +26,6 @@ from tools.registry import tool_error logger = logging.getLogger(__name__) -CONNECTOR_ACTIONS = ("status", "connect", "reconnect", "wait") -MCP_ACTIONS = ("install", "enable", "authorize") -ALL_ACTIONS = CONNECTOR_ACTIONS + MCP_ACTIONS - -_TARGET_FIELDS = frozenset({"name", "mcp"}) - # Renderer outcome → operation state. declined = Not now; error = recoverable, operation stays open. _OUTCOME_STATES = { "installed": CONNECTED, "enabled": CONNECTED, "authorized": CONNECTED, "connected": CONNECTED, @@ -42,66 +36,8 @@ _OUTCOME_STATES = { UNAVAILABLE_HINT = "hermes mcp install {name} / hermes mcp login {name}" -def normalize_targets(raw: Any) -> Tuple[List[str], List[str], Optional[str]]: - """``connectors`` → ``(managed names, mcp names, error)``. Bare strings and ``{name}`` are - managed; ``{name, mcp: true}`` is a local MCP. Any other field is an error.""" - if raw is None: - return [], [], None - if isinstance(raw, (str, dict)): - raw = [raw] - if not isinstance(raw, list): - return [], [], "'connectors' must be a list of names or {name, mcp} objects." - managed: List[str] = [] - mcp: List[str] = [] - for item in raw: - if isinstance(item, dict): - unknown = sorted(set(item) - _TARGET_FIELDS) - if unknown: - return [], [], ( - f"unknown target field(s) {', '.join(unknown)}: a target is " - "{\"name\": \"\"} or {\"name\": \"\", \"mcp\": true}. Transport, " - "URLs and credentials come from the catalog manifest, never from the call." - ) - name = str(item.get("name") or "").strip().lower() - is_mcp = bool(item.get("mcp", False)) - else: - name, is_mcp = str(item or "").strip().lower(), False - if not name: - return [], [], "every target needs a non-empty 'name'." - bucket = mcp if is_mcp else managed - if name not in bucket: - bucket.append(name) - return managed, mcp, None -def validate_action(action: str, managed: List[str], mcp: List[str]) -> Optional[str]: - """MCP verbs need ``mcp:true`` targets; connector verbs need managed targets.""" - if action not in ALL_ACTIONS: - return ( - f"action must be one of {', '.join(ALL_ACTIONS)}. " - f"{', '.join(MCP_ACTIONS)} apply to local MCP servers " - "(targets {\"name\": ..., \"mcp\": true}); the rest apply to managed connectors. " - "Disconnecting an account is done by the user in the Nous Portal dashboard, not " - "through this tool." - ) - if action in MCP_ACTIONS: - if managed: - return ( - f"'{action}' is an MCP action: every target must carry \"mcp\": true " - f"(got managed connector(s) {', '.join(managed)}). Managed connectors use " - "connect / reconnect / wait / status." - ) - if not mcp: - return ( - f"'{action}' requires 'connectors': the MCP server name(s), e.g. " - "[{\"name\": \"linear\", \"mcp\": true}]." - ) - elif mcp: - return ( - f"'{action}' is a managed-connector action; MCP targets ({', '.join(mcp)}) use " - f"{', '.join(MCP_ACTIONS)}." - ) - return None def _catalog_names() -> List[str]: diff --git a/tools/connections_tool_operation.py b/tools/connectors/operation.py similarity index 100% rename from tools/connections_tool_operation.py rename to tools/connectors/operation.py diff --git a/tools/connector_search.py b/tools/connectors/search.py similarity index 95% rename from tools/connector_search.py rename to tools/connectors/search.py index e32074f75f..d83e82eab6 100644 --- a/tools/connector_search.py +++ b/tools/connectors/search.py @@ -12,7 +12,7 @@ from __future__ import annotations import logging from typing import Any, Dict, Iterable, List, Optional -from tools.tool_gateway.names import format_connector_name, is_connector_name, vendor_slug_candidates +from tools.connectors.gateway.names import format_connector_name, is_connector_name, vendor_slug_candidates from tools.tool_search_catalog import CatalogEntry, _fn, _tokenize logger = logging.getLogger(__name__) @@ -50,7 +50,7 @@ def connector_entries_by_group( per_query: List[List[CatalogEntry]] = [[] for _ in queries] try: if connector_search is None: - from tools.tool_gateway.bridge import connector_search_hits as connector_search + from tools.connectors.gateway.bridge import connector_search_hits as connector_search hits = connector_search([{"use_case": q} for q in queries]) or {} schemas = hits.get("schemas") groups = hits.get("results") @@ -119,7 +119,7 @@ def remote_schemas_for( return {} try: if connector_describe is None: - from tools.tool_gateway.bridge import connector_describe + from tools.connectors.gateway.bridge import connector_describe remote = connector_describe(connector_names) if isinstance(remote, dict) and isinstance(remote.get("tools"), dict): return remote["tools"] diff --git a/tools/connectors/targets.py b/tools/connectors/targets.py new file mode 100644 index 0000000000..fdeace0fb8 --- /dev/null +++ b/tools/connectors/targets.py @@ -0,0 +1,78 @@ +"""Target and action validation shared by every leg of ``manage_connections``. + +``connectors`` entries are bare slugs or ``{name, mcp}`` objects; bare strings and ``{name}`` are +managed connectors, ``{name, mcp: true}`` is a local MCP server. Connector verbs need managed +targets, MCP verbs need MCP targets. Pure functions, no I/O. +""" + +from __future__ import annotations + +from typing import Any, List, Optional, Tuple + +CONNECTOR_ACTIONS = ("status", "connect", "reconnect", "wait") +MCP_ACTIONS = ("install", "enable", "authorize") +ALL_ACTIONS = CONNECTOR_ACTIONS + MCP_ACTIONS + +_TARGET_FIELDS = frozenset({"name", "mcp"}) + + +def normalize_targets(raw: Any) -> Tuple[List[str], List[str], Optional[str]]: + """``connectors`` → ``(managed names, mcp names, error)``. Bare strings and ``{name}`` are + managed; ``{name, mcp: true}`` is a local MCP. Any other field is an error.""" + if raw is None: + return [], [], None + if isinstance(raw, (str, dict)): + raw = [raw] + if not isinstance(raw, list): + return [], [], "'connectors' must be a list of names or {name, mcp} objects." + managed: List[str] = [] + mcp: List[str] = [] + for item in raw: + if isinstance(item, dict): + unknown = sorted(set(item) - _TARGET_FIELDS) + if unknown: + return [], [], ( + f"unknown target field(s) {', '.join(unknown)}: a target is " + "{\"name\": \"\"} or {\"name\": \"\", \"mcp\": true}. Transport, " + "URLs and credentials come from the catalog manifest, never from the call." + ) + name = str(item.get("name") or "").strip().lower() + is_mcp = bool(item.get("mcp", False)) + else: + name, is_mcp = str(item or "").strip().lower(), False + if not name: + return [], [], "every target needs a non-empty 'name'." + bucket = mcp if is_mcp else managed + if name not in bucket: + bucket.append(name) + return managed, mcp, None + + +def validate_action(action: str, managed: List[str], mcp: List[str]) -> Optional[str]: + """MCP verbs need ``mcp:true`` targets; connector verbs need managed targets.""" + if action not in ALL_ACTIONS: + return ( + f"action must be one of {', '.join(ALL_ACTIONS)}. " + f"{', '.join(MCP_ACTIONS)} apply to local MCP servers " + "(targets {\"name\": ..., \"mcp\": true}); the rest apply to managed connectors. " + "Disconnecting an account is done by the user in the Nous Portal dashboard, not " + "through this tool." + ) + if action in MCP_ACTIONS: + if managed: + return ( + f"'{action}' is an MCP action: every target must carry \"mcp\": true " + f"(got managed connector(s) {', '.join(managed)}). Managed connectors use " + "connect / reconnect / wait / status." + ) + if not mcp: + return ( + f"'{action}' requires 'connectors': the MCP server name(s), e.g. " + "[{\"name\": \"linear\", \"mcp\": true}]." + ) + elif mcp: + return ( + f"'{action}' is a managed-connector action; MCP targets ({', '.join(mcp)}) use " + f"{', '.join(MCP_ACTIONS)}." + ) + return None diff --git a/tools/connectors/tool.py b/tools/connectors/tool.py new file mode 100644 index 0000000000..3d6c50884a --- /dev/null +++ b/tools/connectors/tool.py @@ -0,0 +1,175 @@ +#!/usr/bin/env python3 +"""Manage remote connector accounts served through the tool gateway. + +``manage_connections`` is the never-deferred surface for connection +lifecycle: + +- ``status`` — which connectors exist for this account and whether each is + connected (read-only). +- ``connect`` / ``reconnect`` — start (or restart) an authorization flow. + The gateway returns a connect link, passed through UN-redacted: the model + shows it to the user, who opens it in a browser. Each connector's + ``instruction`` text is surfaced once per session, not on every call. +- ``wait`` — block inside the call until the named connectors report + connected, or the budget runs out. A model has no clock: told to wait it + says "I'll check back in a minute" and its next action lands immediately, + so guidance produced a burst of polls rather than a paced one. Waiting + inside the call cannot be skipped and works the same on every platform. + +Scope: managed connectors AND local MCP servers. ``{"name": ..., "mcp": true}`` targets take +``install`` / ``enable`` / ``authorize`` through one connection operation (``operation.py``, +``mcp.py``); the approval card is reached only via the inline executor, so non-GUI surfaces get +``unavailable`` for MCP targets. Managed actions live in ``managed.py``; target and action +validation in ``targets.py``. + +De-authentication is deliberately NOT exposed to the model: disconnecting +an account is a user decision, made in the portal dashboard. + +Availability: the managed leg is portal-gated in the handler; MCP targets need no sign-in. +""" + +from typing import Any, Callable, Dict, Optional + +from tools.connectors.gateway import config as gateway_config +from tools.connectors.managed import ( + _WAIT_DEFAULT_SECONDS, + _WAIT_MAX_SECONDS, + _WAIT_MIN_SECONDS, + run_managed_action, +) +from tools.connectors.mcp import run_mcp_operation +from tools.connectors.targets import ALL_ACTIONS, MCP_ACTIONS, normalize_targets, validate_action +from tools.registry import registry, tool_error + + + +def manage_connections( + args: Dict[str, Any], + *, + client_factory: Optional[Callable[[], Any]] = None, + seen_instructions: Optional[set] = None, + rendered_links: Optional[Dict[str, Dict[str, float]]] = None, + session_id: Optional[str] = None, + connection_callback: Optional[Callable[[Dict[str, Any]], Optional[str]]] = None, + connectors_available: Optional[Callable[[], bool]] = None, + wait_seconds: Optional[float] = None, +) -> str: + """Dispatch one ``manage_connections`` action. Returns a JSON string.""" + action = str(args.get("action") or "status").strip().lower() + managed, mcp_targets, target_error = normalize_targets(args.get("connectors")) + if target_error: + return tool_error(target_error) + action_error = validate_action(action, managed, mcp_targets) + if action_error: + return tool_error(action_error) + + if action in MCP_ACTIONS: + return run_mcp_operation( + mcp_targets, action, str(args.get("reason") or "").strip(), + connection_callback=connection_callback, session_id=session_id, wait_seconds=wait_seconds, + ) + + return run_managed_action( + action, managed, args, + client_factory=client_factory, seen_instructions=seen_instructions, + rendered_links=rendered_links, session_id=session_id, + connectors_available=connectors_available, + ) + +MANAGE_CONNECTIONS_SCHEMA = { + "name": "manage_connections", + "description": ( + "Connect the user to apps: managed connector accounts (Gmail, Notion, ...) served " + "through the tool gateway, and local MCP servers from the catalog. Targets go in " + "'connectors': a bare slug or {\"name\": \"gmail\"} is a managed connector; " + "{\"name\": \"linear\", \"mcp\": true} is a local MCP server. " + "Managed actions: 'status' lists connectors and whether each is connected; 'connect' " + "starts an authorization for the given connectors and returns a link " + "for the USER to open in a browser (never open it yourself); " + "'reconnect' restarts a broken authorization; " + "'wait' blocks until the given connectors report connected. Pass " + "SEVERAL slugs in one call to get all authorization links at once. " + "When a connector tool " + "call returns CONNECTION_REQUIRED, use 'connect' and show the link. " + "Send the message that shows the user the links FIRST; on your NEXT " + "turn call 'wait' with those same slugs instead of guessing when the " + "user is done — it polls for you (a wait in the same turn as the " + "connect is bounced, because the user cannot have seen the links " + "yet). 'wait' requires 'connectors', and only accepts connectors this " + "session already addressed with 'connect' (already-connected apps " + "count). A 'timeout' or 'interrupted' result is NOT an " + "error: the user has not finished connecting, so ask them whether to " + "keep waiting, continue without those apps, or get fresh links. " + "MCP actions (targets must carry \"mcp\": true): 'install' adds a catalog entry, " + "'enable' re-enables a disabled configured server, 'authorize' runs its OAuth. " + "They show the user an approval card and block until it settles; the result lists " + "each target as connected / skipped / not_connected. Never hand-edit mcp_servers " + "config — always use this tool. Never re-ask after a skip or timeout: continue " + "without the server or ask in chat. A newly installed or authorized server's tools " + "arrive on your next turn. Off the desktop app the MCP targets come back " + "'unavailable' with the terminal commands to give the user. " + "This tool can NOT disconnect, delete, or revoke an account — that is " + "deliberately user-only. When asked, say so and direct the user to " + "the Nous Portal (their org's Connectors page) or the desktop app." + ), + "parameters": { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": list(ALL_ACTIONS), + "description": "Defaults to status. install/enable/authorize need mcp:true targets.", + }, + "connectors": { + "type": "array", + "items": { + "anyOf": [ + {"type": "string"}, + { + "type": "object", + "properties": { + "name": {"type": "string"}, + "mcp": {"type": "boolean", "description": "true = local MCP server."}, + }, + "required": ["name"], + "additionalProperties": False, + }, + ] + }, + "description": ( + "Targets. REQUIRED for every action but status " + "(e.g. [\"gmail\", {\"name\": \"linear\", \"mcp\": true}]); optional filter for status." + ), + }, + "reason": { + "type": "string", + "description": "MCP actions: one sentence on the approval card — why this helps right now.", + }, + "timeout_seconds": { + "type": "integer", + "description": ( + "For action 'wait' only: how long to hold the call open. " + f"Defaults to {int(_WAIT_DEFAULT_SECONDS)}, clamped to " + f"{int(_WAIT_MIN_SECONDS)}-{int(_WAIT_MAX_SECONDS)}. Ask for " + "more and the result carries a 'timeout_note' saying the cap " + "was applied; call wait again to keep waiting." + ), + }, + }, + "required": [], + }, +} + + +registry.register( + name="manage_connections", + toolset="connections", + schema=MANAGE_CONNECTIONS_SCHEMA, + # The portal gate is in the handler, not check_fn, so signed-out sessions keep the tool for + # MCP approvals. The registry path has no GUI callback. Module attribute, not a bound name, so + # tests patch ``gateway.config.connectors_available`` once and every reader sees it. + handler=lambda args, **kw: manage_connections( + args, session_id=kw.get("session_id"), connectors_available=gateway_config.connectors_available, + ), + emoji="🔗", +) diff --git a/tools/registry.py b/tools/registry.py index f2469d4136..9aa89f613c 100644 --- a/tools/registry.py +++ b/tools/registry.py @@ -83,19 +83,34 @@ def _module_registers_tools(module_path: Path) -> bool: for stmt in tree.body) +def _tool_module_candidates(tools_path: Path) -> List[Path]: + """Flat ``tools/*.py`` modules plus the package entry point ``tools//tool.py``, in one + sorted list. Only ``tool.py`` is scanned in a package, so every other file in it is a library + by construction. Sorted after merging: ``register()`` lets a same-name, same-toolset duplicate + overwrite silently, so the import order must not depend on file depth.""" + candidates = list(tools_path.glob("*.py")) + list(tools_path.glob("*/tool.py")) + return sorted(candidates) + + def discover_builtin_tools(tools_dir: Optional[Path] = None) -> List[str]: """Import built-in self-registering tool modules and return their module names. The per-file AST scan costs ~145 ms over ~100 files, so verdicts are memoized on disk keyed by ``(mtime_ns, size)``; a mismatch or corrupt cache re-scans that file. The write is best-effort and atomic, so concurrent processes race harmlessly.""" - tools_path = Path(tools_dir) if tools_dir is not None else Path(__file__).resolve().parent + tools_path = (Path(tools_dir) if tools_dir is not None else Path(__file__).resolve().parent).resolve() cache = _load_discovery_cache() fresh_cache: Dict[str, list] = {} cache_dirty = False module_names: List[str] = [] - for path in sorted(tools_path.glob("*.py")): + for path in _tool_module_candidates(tools_path): if path.name in {"__init__.py", "registry.py", "mcp_tool.py"}: continue + rel_parts = path.relative_to(tools_path).with_suffix("").parts + if len(rel_parts) > 1 and not (path.parent / "__init__.py").exists(): + # setuptools' package finder drops a directory without __init__.py, so this tool would + # register from a checkout and vanish from an installed wheel. + logger.warning("Skipping %s: package %s has no __init__.py", path, path.parent.name) + continue abs_path = str(path.resolve()) try: st = path.stat() @@ -110,7 +125,7 @@ def discover_builtin_tools(tools_dir: Optional[Path] = None) -> List[str]: cache_dirty = True fresh_cache[abs_path] = [stat_key[0], stat_key[1], registers] if registers: - module_names.append(f"tools.{path.stem}") + module_names.append(".".join(("tools", *rel_parts))) # Drop entries for files that no longer exist; rewrite only when changed. if cache_dirty or set(fresh_cache) != set(cache): diff --git a/tools/tool_search.py b/tools/tool_search.py index ca372a01bf..dedc93929d 100644 --- a/tools/tool_search.py +++ b/tools/tool_search.py @@ -22,8 +22,8 @@ from tools.tool_search_catalog import ( CatalogEntry, _fn, _listing_group_label, _registry_entry, _registry_toolset, build_catalog, build_catalog_listing_with_form, search_catalog) from tools.tool_search_validation import normalize_tool_call_entries, validate_deferred_call_args -from tools.connector_search import connections_in_scope, connector_entries_by_group, remote_schemas_for -from tools.tool_gateway.names import CONNECTOR_BATCH_SENTINEL, is_connector_name +from tools.connectors import CONNECTOR_BATCH_SENTINEL, is_connector_name +from tools.connectors.search import connections_in_scope, connector_entries_by_group, remote_schemas_for logger = logging.getLogger("tools.tool_search") # Bound the work one bridge call requests. Search is capped at the gateway's diff --git a/tui_gateway/methods_connectors.py b/tui_gateway/methods_connectors.py index 771ddf5502..97fd2b2eaa 100644 --- a/tui_gateway/methods_connectors.py +++ b/tui_gateway/methods_connectors.py @@ -82,7 +82,7 @@ def _connector_rpc(rid, params, action): def _dispatch_connector_rpc(rid, sid, owner, profile_home, args): import model_tools - from tools.tool_gateway.config import connectors_available + from tools.connectors import connectors_available from tui_gateway.connector_payload import connector_ui_payload agent = owner.get("agent") diff --git a/tui_gateway/tool_progress.py b/tui_gateway/tool_progress.py index bab6c68f98..5176a3a14f 100644 --- a/tui_gateway/tool_progress.py +++ b/tui_gateway/tool_progress.py @@ -189,7 +189,7 @@ def _todo_state_from_history(history) -> dict | None: def _connector_tool_lifecycle(name: str, args: dict) -> bool: - from tools.tool_gateway.names import is_connector_name + from tools.connectors import is_connector_name if name == "manage_connections" or is_connector_name(name): return True