From 2b03095d4eedb169e46e345e0b2bfb26d2411714 Mon Sep 17 00:00:00 2001 From: huanghuoguoguo <60681390+huanghuoguoguo@users.noreply.github.com> Date: Mon, 22 Jun 2026 13:39:45 +0800 Subject: [PATCH] refactor(tools): unify tool-detail normalization in ToolManager Drop the PluginToolLoader.get_tool() override that returned a raw ComponentManifest, so every loader's get_tool() now returns a uniform resource_tool.LLMTool (PluginToolLoader.get_tools() already did this conversion). This removes the only source of tool-shape heterogeneity. - ToolManager.get_tool_schema(): drop the ComponentManifest-vs-LLMTool branch - ToolManager.get_tool_detail(): new host-level shape {name, description, human_desc, parameters} - handler.py GET_TOOL_DETAIL: call tool_mgr.get_tool_detail(); delete the handler-local _build_tool_detail + _i18n_to_dict/_i18n_to_text adapters and the litellm TODO - ToolLookupResult is now just LLMTool The dropped label/spec fields were not consumed by any runner (local-agent build_llm_tool and external harnesses use only name/description/parameters). --- src/langbot/pkg/plugin/handler.py | 66 +------------------ src/langbot/pkg/provider/tools/loader.py | 4 +- .../pkg/provider/tools/loaders/plugin.py | 7 -- src/langbot/pkg/provider/tools/toolmgr.py | 29 +++++--- tests/unit_tests/agent/test_handler_auth.py | 56 +++++++++------- 5 files changed, 56 insertions(+), 106 deletions(-) diff --git a/src/langbot/pkg/plugin/handler.py b/src/langbot/pkg/plugin/handler.py index 6fd859386..3f79182b8 100644 --- a/src/langbot/pkg/plugin/handler.py +++ b/src/langbot/pkg/plugin/handler.py @@ -62,66 +62,6 @@ def _pop_query_llm_usage(query: Any) -> dict[str, Any] | None: return None -def _i18n_to_dict(value: Any) -> dict[str, Any]: - """Convert SDK i18n values to plain dictionaries.""" - if value is None: - return {} - if isinstance(value, dict): - return value - if hasattr(value, 'to_dict'): - return value.to_dict() - if hasattr(value, 'model_dump'): - return value.model_dump() - return {'en_US': str(value)} - - -def _i18n_to_text(value: Any) -> str: - """Return a stable human-readable text from SDK i18n values.""" - data = _i18n_to_dict(value) - for key in ('en_US', 'zh_Hans', 'zh_Hant'): - text = data.get(key) - if text: - return str(text) - for text in data.values(): - if text: - return str(text) - return '' - - -def _build_tool_detail(tool: Any, requested_tool_name: str | None = None) -> dict[str, Any]: - """Normalize LLMTool and plugin ComponentManifest objects for tool detail APIs.""" - # TODO(litellm): This handler-local adapter is temporary. Once LiteLLM-backed - # tool schema normalization owns tool detail generation, simplify GET_TOOL_DETAIL - # and make ToolManager return one host-level tool detail shape. - if hasattr(tool, 'metadata') and hasattr(tool, 'spec'): - metadata = tool.metadata - spec = tool.spec or {} - description = spec.get('llm_prompt') or _i18n_to_text(getattr(metadata, 'description', None)) - parameters = spec.get('parameters') or {} - - return { - 'name': requested_tool_name or getattr(metadata, 'name', ''), - 'label': _i18n_to_dict(getattr(metadata, 'label', None)), - 'description': description, - 'human_desc': description, - 'parameters': parameters, - 'spec': spec, - } - - name = getattr(tool, 'name', requested_tool_name or '') - description = getattr(tool, 'description', None) or getattr(tool, 'human_desc', '') or '' - parameters = getattr(tool, 'parameters', None) or {} - - return { - 'name': name, - 'label': {}, - 'description': description, - 'human_desc': getattr(tool, 'human_desc', description) or description, - 'parameters': parameters, - 'spec': {'parameters': parameters}, - } - - def _normalize_uuid_list(values: Any) -> list[str]: """Normalize a user/config supplied UUID list while preserving order.""" if not isinstance(values, list): @@ -761,14 +701,12 @@ class RuntimeConnectionHandler(handler.Handler): return error try: - tool = await self.ap.tool_mgr.get_tool_by_name(tool_name) - if tool is None: + tool_detail = await self.ap.tool_mgr.get_tool_detail(tool_name) + if tool_detail is None: return handler.ActionResponse.error( message=f'Tool {tool_name} not found', ) - tool_detail = _build_tool_detail(tool, requested_tool_name=tool_name) - return handler.ActionResponse.success(data={'tool': tool_detail}) except Exception as e: traceback.print_exc() diff --git a/src/langbot/pkg/provider/tools/loader.py b/src/langbot/pkg/provider/tools/loader.py index f97e82164..22b4a305b 100644 --- a/src/langbot/pkg/provider/tools/loader.py +++ b/src/langbot/pkg/provider/tools/loader.py @@ -4,14 +4,14 @@ import abc import typing from typing import TYPE_CHECKING -from langbot_plugin.api.definition.components.manifest import ComponentManifest from langbot_plugin.api.entities.events import pipeline_query import langbot_plugin.api.entities.builtin.resource.tool as resource_tool if TYPE_CHECKING: from ...core import app -ToolLookupResult = resource_tool.LLMTool | ComponentManifest +# All loaders normalize their tools to resource_tool.LLMTool. +ToolLookupResult = resource_tool.LLMTool preregistered_loaders: list[typing.Type[ToolLoader]] = [] diff --git a/src/langbot/pkg/provider/tools/loaders/plugin.py b/src/langbot/pkg/provider/tools/loaders/plugin.py index 544882d34..44f40e421 100644 --- a/src/langbot/pkg/provider/tools/loaders/plugin.py +++ b/src/langbot/pkg/provider/tools/loaders/plugin.py @@ -3,7 +3,6 @@ from __future__ import annotations import typing import traceback -from langbot_plugin.api.definition.components.manifest import ComponentManifest from langbot_plugin.api.entities.events import pipeline_query from .. import loader @@ -40,12 +39,6 @@ class PluginToolLoader(loader.ToolLoader): return True return False - async def get_tool(self, name: str) -> ComponentManifest | None: - for tool in await self.ap.plugin_connector.list_tools(): - if tool.metadata.name == name: - return tool - return None - async def invoke_tool(self, name: str, parameters: dict, query: pipeline_query.Query) -> typing.Any: try: return await self.ap.plugin_connector.call_tool( diff --git a/src/langbot/pkg/provider/tools/toolmgr.py b/src/langbot/pkg/provider/tools/toolmgr.py index ad73cf567..41bf31f6a 100644 --- a/src/langbot/pkg/provider/tools/toolmgr.py +++ b/src/langbot/pkg/provider/tools/toolmgr.py @@ -90,18 +90,31 @@ class ToolManager: """Return (description, parameters JSON schema) for a tool by name. Used by the host to prefill ToolResource so a runner can build LLM tool - definitions without a separate get_tool_detail round-trip. Handles both - LLMTool (native/mcp/skill) and plugin ComponentManifest shapes. Returns - (None, None) when the tool is not found. + definitions without a separate get_tool_detail round-trip. All loaders + return resource_tool.LLMTool, so no per-shape branching is needed. + Returns (None, None) when the tool is not found. """ tool = await self.get_tool_by_name(name) if tool is None: return None, None - if hasattr(tool, 'spec') and hasattr(tool, 'metadata'): - spec = getattr(tool, 'spec', None) or {} - return spec.get('llm_prompt'), (spec.get('parameters') or None) - description = getattr(tool, 'description', None) or getattr(tool, 'human_desc', None) - return description, (getattr(tool, 'parameters', None) or None) + return tool.description, (tool.parameters or None) + + async def get_tool_detail(self, name: str) -> dict | None: + """Return the host-level tool detail shape for a tool by name. + + All loaders return resource_tool.LLMTool, so the shape is uniform: + {name, description, human_desc, parameters}. Returns None when the tool + is not found. + """ + tool = await self.get_tool_by_name(name) + if tool is None: + return None + return { + 'name': tool.name, + 'description': tool.description, + 'human_desc': tool.human_desc, + 'parameters': tool.parameters or {}, + } async def generate_tools_for_openai(self, use_funcs: list[resource_tool.LLMTool]) -> list: tools = [] diff --git a/tests/unit_tests/agent/test_handler_auth.py b/tests/unit_tests/agent/test_handler_auth.py index 18516951e..d7865d450 100644 --- a/tests/unit_tests/agent/test_handler_auth.py +++ b/tests/unit_tests/agent/test_handler_auth.py @@ -14,12 +14,11 @@ Authorization paths: from __future__ import annotations import pytest -import types from unittest.mock import AsyncMock, MagicMock from langbot.pkg.agent.runner.descriptor import AgentRunnerDescriptor from langbot.pkg.agent.runner.session_registry import AgentRunSessionRegistry -from langbot.pkg.plugin.handler import _build_tool_detail, _get_pipeline_knowledge_base_uuids +from langbot.pkg.plugin.handler import _get_pipeline_knowledge_base_uuids # Import shared test fixtures from conftest.py from .conftest import make_resources, make_session @@ -287,31 +286,39 @@ class TestInvokeLLMStreamAuthorization: assert run_id is None -def test_build_tool_detail_normalizes_plugin_component_manifest(): - """GET_TOOL_DETAIL returns a uniform schema for ordinary plugin Tool manifests.""" - manifest_tool = types.SimpleNamespace( - metadata=types.SimpleNamespace( - name='search', - label={'en_US': 'Search'}, - description={'en_US': 'Search public data'}, - ), - spec={ - 'llm_prompt': 'Search test data', - 'parameters': { - 'type': 'object', - 'properties': {'q': {'type': 'string'}}, - }, - }, +@pytest.mark.asyncio +async def test_tool_manager_get_tool_detail_returns_uniform_schema(): + """ToolManager.get_tool_detail returns a uniform host-level tool detail shape. + + All loaders normalize to resource_tool.LLMTool, so GET_TOOL_DETAIL no longer + needs a handler-local adapter for plugin ComponentManifest objects. + """ + import langbot_plugin.api.entities.builtin.resource.tool as resource_tool + from langbot.pkg.provider.tools.toolmgr import ToolManager + + tool = resource_tool.LLMTool( + name='search', + human_desc='Search public data', + description='Search test data', + parameters={'type': 'object', 'properties': {'q': {'type': 'string'}}}, + func=lambda **kwargs: {}, ) - detail = _build_tool_detail(manifest_tool, requested_tool_name='author/plugin/search') + mgr = ToolManager.__new__(ToolManager) - assert detail['name'] == 'author/plugin/search' - assert detail['description'] == 'Search test data' - assert detail['human_desc'] == 'Search test data' - assert detail['parameters']['properties']['q']['type'] == 'string' - assert detail['label'] == {'en_US': 'Search'} - assert detail['spec'] == manifest_tool.spec + async def fake_get_tool_by_name(name): + return tool if name == 'search' else None + + mgr.get_tool_by_name = fake_get_tool_by_name + + detail = await mgr.get_tool_detail('search') + assert detail == { + 'name': 'search', + 'description': 'Search test data', + 'human_desc': 'Search public data', + 'parameters': {'type': 'object', 'properties': {'q': {'type': 'string'}}}, + } + assert await mgr.get_tool_detail('missing') is None class TestCallToolAuthorization: @@ -1510,7 +1517,6 @@ class TestStorageResourcePermissionHelper: assert registry.is_resource_allowed(session, 'storage', 'workspace') is False - class TestRealActionHandlerSimulation: """Tests that simulate real RuntimeConnectionHandler action registration and execution.