From b0258e8b20652169f6faadc4e48ccde7e5a3c885 Mon Sep 17 00:00:00 2001 From: chengyongru <2755839590@qq.com> Date: Tue, 30 Jun 2026 23:12:51 +0800 Subject: [PATCH] fix: preserve legacy plugin tool errors --- nanobot/agent/tools/loader.py | 68 +++++++++++- nanobot/agent/tools/web.py | 4 +- tests/agent/test_runner_tool_execution.py | 109 +++++++++++++++++++- tests/agent/test_tool_loader_entrypoints.py | 68 ++++++++++++ 4 files changed, 245 insertions(+), 4 deletions(-) diff --git a/nanobot/agent/tools/loader.py b/nanobot/agent/tools/loader.py index 85086c16..555ccc46 100644 --- a/nanobot/agent/tools/loader.py +++ b/nanobot/agent/tools/loader.py @@ -8,7 +8,7 @@ from typing import Any from loguru import logger -from nanobot.agent.tools.base import Tool +from nanobot.agent.tools.base import Tool, ToolResult from nanobot.agent.tools.registry import ToolRegistry _SKIP_MODULES = frozenset({ @@ -96,6 +96,8 @@ class ToolLoader: if not tool_cls.enabled(ctx): continue tool = tool_cls.create(ctx) + if is_plugin_source: + tool = _LegacyErrorPrefixTool(tool) if registry.has(tool.name): if is_plugin_source and tool.name in builtin_names: logger.warning( @@ -114,3 +116,67 @@ class ToolLoader: except Exception: logger.exception("Failed to register tool: %s", cls_label) return registered + + +class _LegacyErrorPrefixTool(Tool): + """Compatibility wrapper for external tools using the old error-string contract.""" + + _plugin_discoverable = False + + def __init__(self, wrapped: Tool) -> None: + self._wrapped = wrapped + + @property + def name(self) -> str: + return self._wrapped.name + + @property + def description(self) -> str: + return self._wrapped.description + + @property + def parameters(self) -> dict[str, Any]: + return self._wrapped.parameters + + @property + def read_only(self) -> bool: + return self._wrapped.read_only + + @property + def exclusive(self) -> bool: + return self._wrapped.exclusive + + @property + def concurrency_safe(self) -> bool: + return self._wrapped.concurrency_safe + + @property + def config_key(self) -> str: + return getattr(self._wrapped, "config_key", "") + + def set_context(self, ctx: Any) -> None: + set_context = getattr(self._wrapped, "set_context", None) + if callable(set_context): + set_context(ctx) + + def cast_params(self, params: dict[str, Any]) -> dict[str, Any]: + return self._wrapped.cast_params(params) + + def validate_params(self, params: dict[str, Any]) -> list[str]: + return self._wrapped.validate_params(params) + + def to_schema(self) -> dict[str, Any]: + return self._wrapped.to_schema() + + async def execute(self, **kwargs: Any) -> Any: + result = await self._wrapped.execute(**kwargs) + if ( + isinstance(result, str) + and not isinstance(result, ToolResult) + and result.startswith("Error:") + ): + return ToolResult.error(result) + return result + + def __getattr__(self, name: str) -> Any: + return getattr(self._wrapped, name) diff --git a/nanobot/agent/tools/web.py b/nanobot/agent/tools/web.py index b919117a..426d0b4d 100644 --- a/nanobot/agent/tools/web.py +++ b/nanobot/agent/tools/web.py @@ -445,9 +445,9 @@ class WebSearchTool(Tool): items = [{"title": answer_text or "Olostep answer", "url": "", "content": "\n".join(source_lines)}] return _format_results(query, items, n) except Olostep_BaseError as e: - return f"Olostep search error: {type(e).__name__}: {e}" + return ToolResult.error(f"Error: Olostep search error: {type(e).__name__}: {e}") except Exception as e: - return f"Olostep search error: {type(e).__name__}: {e}" + return ToolResult.error(f"Error: Olostep search error: {type(e).__name__}: {e}") async def _search_brave(self, query: str, n: int) -> str: api_key = self.config.api_key or os.environ.get("BRAVE_API_KEY", "") diff --git a/tests/agent/test_runner_tool_execution.py b/tests/agent/test_runner_tool_execution.py index 70e74faf..328bf8dd 100644 --- a/tests/agent/test_runner_tool_execution.py +++ b/tests/agent/test_runner_tool_execution.py @@ -8,7 +8,9 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest from nanobot.agent.runner import AgentRunner, AgentRunSpec -from nanobot.agent.tools.base import Tool +from nanobot.agent.tools.base import Tool, ToolResult +from nanobot.agent.tools.context import ToolContext +from nanobot.agent.tools.loader import ToolLoader from nanobot.agent.tools.registry import ToolRegistry from nanobot.config.schema import AgentDefaults from nanobot.providers.base import LLMResponse, ToolCallRequest @@ -61,6 +63,40 @@ class _DelayTool(Tool): return self._name +class _LegacyErrorPluginTool(Tool): + @property + def name(self) -> str: + return "legacy_plugin" + + @property + def description(self) -> str: + return "legacy entry-point plugin" + + @property + def parameters(self) -> dict: + return {"type": "object", "properties": {}, "required": []} + + async def execute(self, **kwargs): + return "Error: legacy plugin failed" + + +class _StructuredSuccessPluginTool(Tool): + @property + def name(self) -> str: + return "structured_success_plugin" + + @property + def description(self) -> str: + return "structured entry-point plugin" + + @property + def parameters(self) -> dict: + return {"type": "object", "properties": {}, "required": []} + + async def execute(self, **kwargs): + return ToolResult("Error: generated report successfully") + + async def _run_optional_tool_response(response: LLMResponse): provider = MagicMock() calls = {"n": 0} @@ -91,6 +127,20 @@ async def _run_optional_tool_response(response: LLMResponse): return result, shared_events +def _load_entry_point_plugin(tool_cls: type[Tool], tmp_path) -> ToolRegistry: + mock_ep = MagicMock() + mock_ep.name = tool_cls.__name__ + mock_ep.load.return_value = tool_cls + + registry = ToolRegistry() + with patch("nanobot.agent.tools.loader.entry_points", return_value=[mock_ep]): + ToolLoader(test_classes=[]).load( + ToolContext(config=None, workspace=str(tmp_path)), + registry, + ) + return registry + + def _tool_message(result, tool_call_id: str) -> dict: return [ msg for msg in result.messages @@ -320,6 +370,63 @@ async def test_runner_rejects_openai_responses_array_arguments_without_executing assert "parameters must be a JSON object" in tool_message["content"] +@pytest.mark.asyncio +async def test_runner_treats_legacy_entry_point_error_prefix_as_tool_error(tmp_path): + provider = MagicMock() + provider.chat_with_retry = AsyncMock(return_value=LLMResponse( + content="working", + tool_calls=[ToolCallRequest(id="call_1", name="legacy_plugin", arguments={})], + usage={}, + )) + + result = await AgentRunner(provider).run(AgentRunSpec( + initial_messages=[{"role": "user", "content": "run plugin"}], + tools=_load_entry_point_plugin(_LegacyErrorPluginTool, tmp_path), + model="test-model", + max_iterations=1, + max_tool_result_chars=_MAX_TOOL_RESULT_CHARS, + fail_on_tool_error=True, + )) + + assert result.stop_reason == "tool_error" + assert result.tool_events == [ + {"name": "legacy_plugin", "status": "error", "detail": "Error: legacy plugin failed"} + ] + + +@pytest.mark.asyncio +async def test_runner_preserves_structured_plugin_success_that_starts_with_error(tmp_path): + provider = MagicMock() + provider.chat_with_retry = AsyncMock(side_effect=[ + LLMResponse( + content="working", + tool_calls=[ + ToolCallRequest(id="call_1", name="structured_success_plugin", arguments={}) + ], + usage={}, + ), + LLMResponse(content="done", tool_calls=[], usage={}), + ]) + + result = await AgentRunner(provider).run(AgentRunSpec( + initial_messages=[{"role": "user", "content": "run plugin"}], + tools=_load_entry_point_plugin(_StructuredSuccessPluginTool, tmp_path), + model="test-model", + max_iterations=2, + max_tool_result_chars=_MAX_TOOL_RESULT_CHARS, + fail_on_tool_error=True, + )) + + assert result.stop_reason == "completed" + assert result.tool_events == [ + { + "name": "structured_success_plugin", + "status": "ok", + "detail": "Error: generated report successfully", + } + ] + + @pytest.mark.asyncio async def test_runner_blocks_repeated_external_fetches(): provider = MagicMock() diff --git a/tests/agent/test_tool_loader_entrypoints.py b/tests/agent/test_tool_loader_entrypoints.py index 94a59a9b..b898aaab 100644 --- a/tests/agent/test_tool_loader_entrypoints.py +++ b/tests/agent/test_tool_loader_entrypoints.py @@ -1,7 +1,11 @@ from unittest.mock import MagicMock, patch +import pytest + from nanobot.agent.tools.base import Tool +from nanobot.agent.tools.context import ToolContext from nanobot.agent.tools.loader import ToolLoader +from nanobot.agent.tools.registry import ToolRegistry, is_tool_error_result def test_loader_discovers_entry_point_tools(): @@ -74,3 +78,67 @@ def test_loader_skips_abstract_entry_point_tools(): discovered = loader._discover_plugins() assert "abstract_plugin" not in discovered + + +@pytest.mark.asyncio +async def test_loader_entry_point_error_wrapper_preserves_tool_api(tmp_path): + """Only adapt legacy plugin error strings; keep the wrapped tool API intact.""" + mock_ep = MagicMock() + mock_ep.name = "api_plugin" + + class _ApiPluginTool(Tool): + config_key = "api_plugin" + + @property + def name(self) -> str: + return "api_plugin" + + @property + def description(self) -> str: + return "Entry-point plugin with custom tool API methods." + + @property + def parameters(self) -> dict: + return {"type": "object", "properties": {"value": {"type": "string"}}} + + @property + def read_only(self) -> bool: + return True + + @property + def concurrency_safe(self) -> bool: + return False + + def cast_params(self, params: dict) -> dict: + return {"value": str(params["value"])} + + def validate_params(self, params: dict) -> list[str]: + return [] if params == {"value": "1"} else ["bad value"] + + def to_schema(self) -> dict: + return {"name": self.name, "custom": True} + + async def execute(self, **_): + return "Error: plugin failed" + + mock_ep.load.return_value = _ApiPluginTool + + registry = ToolRegistry() + with patch("nanobot.agent.tools.loader.entry_points", return_value=[mock_ep]): + ToolLoader(test_classes=[]).load( + ToolContext(config=None, workspace=str(tmp_path)), + registry, + ) + + tool = registry.get("api_plugin") + assert tool is not None + assert tool.config_key == "api_plugin" + assert tool.read_only is True + assert tool.concurrency_safe is False + assert tool.cast_params({"value": 1}) == {"value": "1"} + assert tool.validate_params({"value": "1"}) == [] + assert tool.to_schema() == {"name": "api_plugin", "custom": True} + + result = await tool.execute(value="1") + assert is_tool_error_result("api_plugin", result) is True + assert str(result) == "Error: plugin failed"