diff --git a/nanobot/agent/runner.py b/nanobot/agent/runner.py index 784d3271..645e1f48 100644 --- a/nanobot/agent/runner.py +++ b/nanobot/agent/runner.py @@ -18,7 +18,7 @@ from nanobot.agent.context_governance import ( ContextGovernor, ) from nanobot.agent.hook import AgentHook, AgentHookContext, AgentRunHookContext -from nanobot.agent.tools.registry import ToolRegistry +from nanobot.agent.tools.registry import ToolRegistry, is_tool_error_result from nanobot.providers.base import LLMProvider, LLMResponse, ToolCallRequest from nanobot.utils.file_edit_events import ( StreamingFileEditTracker, @@ -1266,7 +1266,7 @@ class AgentRunner: return payload, event, exc return payload, event, None - if isinstance(result, str) and result.startswith("Error"): + if is_tool_error_result(tool_call.name, result): if file_edit_trackers and progress_callback is not None: await invoke_file_edit_progress( progress_callback, diff --git a/nanobot/agent/tools/__init__.py b/nanobot/agent/tools/__init__.py index e94d3a00..882a482b 100644 --- a/nanobot/agent/tools/__init__.py +++ b/nanobot/agent/tools/__init__.py @@ -1,6 +1,6 @@ """Agent tools module.""" -from nanobot.agent.tools.base import Schema, Tool, tool_parameters +from nanobot.agent.tools.base import Schema, Tool, ToolResult, tool_parameters from nanobot.agent.tools.context import ToolContext from nanobot.agent.tools.loader import ToolLoader from nanobot.agent.tools.registry import ToolRegistry @@ -25,6 +25,7 @@ __all__ = [ "Tool", "ToolContext", "ToolLoader", + "ToolResult", "ToolRegistry", "tool_parameters", "tool_parameters_schema", diff --git a/nanobot/agent/tools/apply_patch.py b/nanobot/agent/tools/apply_patch.py index cb6c47a2..8e94c596 100644 --- a/nanobot/agent/tools/apply_patch.py +++ b/nanobot/agent/tools/apply_patch.py @@ -7,7 +7,7 @@ from dataclasses import dataclass from pathlib import Path from typing import Any -from nanobot.agent.tools.base import tool_parameters +from nanobot.agent.tools.base import ToolResult, tool_parameters from nanobot.agent.tools.filesystem import _FsTool from nanobot.agent.tools.schema import ( ArraySchema, @@ -289,8 +289,8 @@ class ApplyPatchTool(_FsTool): _format_summary(summary) for summary in summaries ) except PermissionError as exc: - return f"Error: {exc}" + return ToolResult.error(f"Error: {exc}") except _PatchError as exc: - return f"Error applying patch: {exc}" + return ToolResult.error(f"Error applying patch: {exc}") except Exception as exc: - return f"Error applying patch: {exc}" + return ToolResult.error(f"Error applying patch: {exc}") diff --git a/nanobot/agent/tools/base.py b/nanobot/agent/tools/base.py index da715dc5..92e833f0 100644 --- a/nanobot/agent/tools/base.py +++ b/nanobot/agent/tools/base.py @@ -128,6 +128,21 @@ class Schema(ABC): return Schema.validate_json_schema_value(value, self.to_json_schema(), path) +class ToolResult(str): + """String-compatible tool output with structured status.""" + + is_error: bool + + def __new__(cls, content: str, *, is_error: bool = False) -> ToolResult: + obj = str.__new__(cls, content) + obj.is_error = is_error + return obj + + @classmethod + def error(cls, content: str) -> ToolResult: + return cls(content, is_error=True) + + class Tool(ABC): """Agent capability: read files, run commands, etc.""" @@ -193,9 +208,13 @@ class Tool(ABC): @abstractmethod async def execute(self, **kwargs: Any) -> Any: - """Run the tool; returns a string or list of content blocks.""" + """Run the tool; return content, or ``ToolResult.error(...)`` for failures.""" ... + @staticmethod + def error(content: str) -> ToolResult: + return ToolResult.error(content) + def _cast_object(self, obj: Any, schema: dict[str, Any]) -> dict[str, Any]: if not isinstance(obj, dict): return obj diff --git a/nanobot/agent/tools/cli_apps.py b/nanobot/agent/tools/cli_apps.py index 4fec51e1..8a1430d3 100644 --- a/nanobot/agent/tools/cli_apps.py +++ b/nanobot/agent/tools/cli_apps.py @@ -7,7 +7,7 @@ from typing import Any from pydantic import Field -from nanobot.agent.tools.base import Tool, tool_parameters +from nanobot.agent.tools.base import Tool, ToolResult, tool_parameters from nanobot.agent.tools.schema import ( ArraySchema, BooleanSchema, @@ -136,4 +136,4 @@ class CliAppsTool(Tool): restrict_to_workspace=access.restrict_to_workspace, ) except CliAppError as exc: - return f"Error: {exc.message}" + return ToolResult.error(f"Error: {exc.message}") diff --git a/nanobot/agent/tools/cron.py b/nanobot/agent/tools/cron.py index 29ac52a9..b0098c07 100644 --- a/nanobot/agent/tools/cron.py +++ b/nanobot/agent/tools/cron.py @@ -6,7 +6,7 @@ from contextvars import ContextVar from datetime import datetime from typing import Any -from nanobot.agent.tools.base import Tool, tool_parameters +from nanobot.agent.tools.base import Tool, ToolResult, tool_parameters from nanobot.agent.tools.context import ContextAware, RequestContext from nanobot.agent.tools.schema import ( IntegerSchema, @@ -99,7 +99,7 @@ class CronTool(Tool, ContextAware): try: ZoneInfo(tz) except (KeyError, Exception): - return f"Error: unknown timezone '{tz}'" + return ToolResult.error(f"Error: unknown timezone '{tz}'") return None def _display_timezone(self, schedule: CronSchedule) -> str: @@ -148,7 +148,7 @@ class CronTool(Tool, ContextAware): ) -> str: if action == "add": if self._in_cron_context.get(): - return "Error: cannot schedule new jobs from within a cron job execution" + return ToolResult.error("Error: cannot schedule new jobs from within a cron job execution") return self._add_job(name, message, every_seconds, cron_expr, tz, at) elif action == "list": return self._list_jobs() @@ -166,20 +166,20 @@ class CronTool(Tool, ContextAware): at: str | None, ) -> str: if not message: - return ( + return ToolResult.error( "Error: cron action='add' requires a non-empty 'message' parameter " "describing what to do when the job triggers " "(e.g. the reminder text). Retry including message=\"...\"." ) session_key = self._session_key.get() if not session_key: - return "Error: scheduled cron jobs must be created from a chat session" + return ToolResult.error("Error: scheduled cron jobs must be created from a chat session") origin_channel = self._origin_channel.get() origin_chat_id = self._origin_chat_id.get() if not origin_channel or not origin_chat_id: - return "Error: scheduled cron jobs must be created from a chat session" + return ToolResult.error("Error: scheduled cron jobs must be created from a chat session") if tz and not cron_expr: - return "Error: tz can only be used with cron_expr" + return ToolResult.error("Error: tz can only be used with cron_expr") if tz: if err := self._validate_timezone(tz): return err @@ -199,7 +199,7 @@ class CronTool(Tool, ContextAware): try: dt = datetime.fromisoformat(at) except ValueError: - return f"Error: invalid ISO datetime format '{at}'. Expected format: YYYY-MM-DDTHH:MM:SS" + return ToolResult.error(f"Error: invalid ISO datetime format '{at}'. Expected format: YYYY-MM-DDTHH:MM:SS") if dt.tzinfo is None: if err := self._validate_timezone(self._default_timezone): return err @@ -208,7 +208,7 @@ class CronTool(Tool, ContextAware): schedule = CronSchedule(kind="at", at_ms=at_ms) delete_after = True else: - return "Error: either every_seconds, cron_expr, or at is required" + return ToolResult.error("Error: either every_seconds, cron_expr, or at is required") job = self._cron.add_job( name=name or message[:30], @@ -279,7 +279,7 @@ class CronTool(Tool, ContextAware): def _remove_job(self, job_id: str | None) -> str: if not job_id: - return "Error: job_id is required for remove" + return ToolResult.error("Error: job_id is required for remove") result = self._cron.remove_job(job_id) if result == "removed": return f"Removed job {job_id}" diff --git a/nanobot/agent/tools/exec_session.py b/nanobot/agent/tools/exec_session.py index b0d79978..e4fdc17b 100644 --- a/nanobot/agent/tools/exec_session.py +++ b/nanobot/agent/tools/exec_session.py @@ -9,7 +9,7 @@ from contextlib import suppress from dataclasses import dataclass from typing import Any -from nanobot.agent.tools.base import Tool, tool_parameters +from nanobot.agent.tools.base import Tool, ToolResult, tool_parameters from nanobot.agent.tools.context import current_request_session_key from nanobot.agent.tools.schema import ( BooleanSchema, @@ -492,11 +492,12 @@ class WriteStdinTool(Tool): max_output_chars=output_limit, owner_session_key=current_request_session_key(), ) - return format_session_poll(session_id, poll) + result = format_session_poll(session_id, poll) + return ToolResult.error(result) if poll.timed_out else result except KeyError: - return f"Error: exec session not found: {session_id}" + return ToolResult.error(f"Error: exec session not found: {session_id!r}") except Exception as exc: - return f"Error writing to exec session: {exc}" + return ToolResult.error(f"Error writing to exec session: {exc}") async def _wait_for_output( self, @@ -532,13 +533,14 @@ class WriteStdinTool(Tool): joined = "".join(aggregate) if wait_for in joined: poll.output = joined - return format_session_poll(session_id, poll) + result = format_session_poll(session_id, poll) + return ToolResult.error(result) if poll.timed_out else result if poll.done or remaining_ms <= 0: poll.output = "".join(aggregate) result = format_session_poll(session_id, poll) if wait_for not in poll.output: result += f"\nWait target not observed: {wait_for!r}" - return result + return ToolResult.error(result) if poll.timed_out else result @tool_parameters(tool_parameters_schema()) @@ -606,4 +608,4 @@ class ListExecSessionsTool(Tool): ) return "\n".join(lines) except Exception as exc: - return f"Error listing exec sessions: {exc}" + return ToolResult.error(f"Error listing exec sessions: {exc}") diff --git a/nanobot/agent/tools/filesystem.py b/nanobot/agent/tools/filesystem.py index cb1db6ba..4a72d747 100644 --- a/nanobot/agent/tools/filesystem.py +++ b/nanobot/agent/tools/filesystem.py @@ -7,7 +7,7 @@ from dataclasses import dataclass from pathlib import Path from typing import Any -from nanobot.agent.tools.base import Tool, tool_parameters +from nanobot.agent.tools.base import Tool, ToolResult, tool_parameters from nanobot.agent.tools.file_state import FileStates, _hash_file, current_file_states from nanobot.agent.tools.path_utils import resolve_workspace_path from nanobot.agent.tools.schema import ( @@ -268,19 +268,19 @@ class ReadFileTool(_FsTool): ) -> Any: try: if not path: - return "Error reading file: Unknown path" + return ToolResult.error("Error reading file: Unknown path") # Device path blacklist if _is_blocked_device(path): - return f"Error: Reading {path} is blocked (device path that could hang or produce infinite output)." + return ToolResult.error(f"Error: Reading {path} is blocked (device path that could hang or produce infinite output).") fp = self._resolve_read(path) if _is_blocked_device(fp): - return f"Error: Reading {fp} is blocked (device path that could hang or produce infinite output)." + return ToolResult.error(f"Error: Reading {fp} is blocked (device path that could hang or produce infinite output).") if not fp.exists(): - return f"Error: File not found: {path}" + return ToolResult.error(f"Error: File not found: {path}") if not fp.is_file(): - return f"Error: Not a file: {path}" + return ToolResult.error(f"Error: Not a file: {path}") # PDF support if fp.suffix.lower() == ".pdf": @@ -343,7 +343,7 @@ class ReadFileTool(_FsTool): mime = detect_image_mime(raw) or mimetypes.guess_type(path)[0] if mime and mime.startswith("image/"): return build_image_content_blocks(raw, mime, str(fp), f"(Image file: {path})") - return f"Error: Cannot read binary file {path} (MIME: {mime or 'unknown'}). Only UTF-8 text and images are supported." + return ToolResult.error(f"Error: Cannot read binary file {path} (MIME: {mime or 'unknown'}). Only UTF-8 text and images are supported.") # Normalize CRLF -> LF before line-splitting. Primarily a Windows # concern (git checkouts with autocrlf, editors saving CRLF) but @@ -357,7 +357,7 @@ class ReadFileTool(_FsTool): if offset < 1: offset = 1 if offset > total: - return f"Error: offset {offset} is beyond end of file ({total} lines)" + return ToolResult.error(f"Error: offset {offset} is beyond end of file ({total} lines)") start = offset - 1 end = min(start + (limit or self._DEFAULT_LIMIT), total) @@ -381,20 +381,20 @@ class ReadFileTool(_FsTool): self._file_states.record_read(fp, offset=offset, limit=limit) return result except PermissionError as e: - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") except Exception as e: - return f"Error reading file: {e}" + return ToolResult.error(f"Error reading file: {e}") def _read_pdf(self, fp: Path, pages: str | None) -> str: try: import fitz # pymupdf except ImportError: - return "Error: PDF reading requires pymupdf. Install with: pip install pymupdf" + return ToolResult.error("Error: PDF reading requires pymupdf. Install with: pip install pymupdf") try: doc = fitz.open(str(fp)) except Exception as e: - return f"Error reading PDF: {e}" + return ToolResult.error(f"Error reading PDF: {e}") total_pages = len(doc) if pages: @@ -402,10 +402,10 @@ class ReadFileTool(_FsTool): start, end = _parse_page_range(pages, total_pages) except (ValueError, IndexError): doc.close() - return f"Error: Invalid page range '{pages}'. Use format like '1-5'." + return ToolResult.error(f"Error: Invalid page range '{pages}'. Use format like '1-5'.") if start > end or start >= total_pages: doc.close() - return f"Error: Page range '{pages}' is out of bounds (document has {total_pages} pages)." + return ToolResult.error(f"Error: Page range '{pages}' is out of bounds (document has {total_pages} pages).") else: start = 0 end = min(total_pages - 1, self._MAX_PDF_PAGES - 1) @@ -437,10 +437,10 @@ class ReadFileTool(_FsTool): result = extract_text(fp) if result is None: - return f"Error: Unsupported file format: {fp.suffix}" + return ToolResult.error(f"Error: Unsupported file format: {fp.suffix}") if result.startswith("[error:"): - return f"Error reading {fp.suffix.upper()} file: {result}" + return ToolResult.error(f"Error reading {fp.suffix.upper()} file: {result}") if not result: return f"({fp.suffix.upper().lstrip('.')} has no extractable text: {fp})" @@ -492,9 +492,9 @@ class WriteFileTool(_FsTool): self._file_states.record_write(fp) return f"Successfully wrote {len(content)} characters to {fp}" except PermissionError as e: - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") except Exception as e: - return f"Error writing file: {e}" + return ToolResult.error(f"Error writing file: {e}") # --------------------------------------------------------------------------- @@ -830,11 +830,11 @@ class EditFileTool(_FsTool): if new_text is None: raise ValueError("Unknown new_text") if occurrence is not None and occurrence < 1: - return "Error: occurrence must be >= 1." + return ToolResult.error("Error: occurrence must be >= 1.") if line_hint is not None and line_hint < 1: - return "Error: line_hint must be >= 1." + return ToolResult.error("Error: line_hint must be >= 1.") if expected_replacements is not None and expected_replacements < 1: - return "Error: expected_replacements must be >= 1." + return ToolResult.error("Error: expected_replacements must be >= 1.") fp = self._resolve_write(path) @@ -853,14 +853,14 @@ class EditFileTool(_FsTool): except OSError: fsize = 0 if fsize > self._MAX_EDIT_FILE_SIZE: - return f"Error: File too large to edit ({fsize / (1024**3):.1f} GiB). Maximum is 1 GiB." + return ToolResult.error(f"Error: File too large to edit ({fsize / (1024**3):.1f} GiB). Maximum is 1 GiB.") # Create-file: old_text='' but file exists and not empty → reject if old_text == "": raw = fp.read_bytes() content = raw.decode("utf-8") if content.strip(): - return f"Error: Cannot create file — {path} already exists and is not empty." + return ToolResult.error(f"Error: Cannot create file — {path} already exists and is not empty.") fp.write_text(new_text, encoding="utf-8") self._file_states.record_write(fp) return f"Successfully edited {fp}" @@ -878,15 +878,15 @@ class EditFileTool(_FsTool): return self._not_found_msg(old_text, content, path) count = len(matches) if replace_all and occurrence is not None: - return "Error: occurrence cannot be used with replace_all=true." + return ToolResult.error("Error: occurrence cannot be used with replace_all=true.") if replace_all and line_hint is not None: - return "Error: line_hint cannot be used with replace_all=true." + return ToolResult.error("Error: line_hint cannot be used with replace_all=true.") if occurrence is not None and line_hint is not None: - return "Error: line_hint cannot be used with occurrence." + return ToolResult.error("Error: line_hint cannot be used with occurrence.") if count > 1 and not replace_all: if occurrence is not None: if occurrence > count: - return ( + return ToolResult.error( f"Error: occurrence {occurrence} is out of range; " f"old_text appears {count} times." ) @@ -894,7 +894,7 @@ class EditFileTool(_FsTool): nearest = min(matches, key=lambda match: abs(match.line - line_hint)) distance = abs(nearest.line - line_hint) if sum(1 for match in matches if abs(match.line - line_hint) == distance) > 1: - return ( + return ToolResult.error( f"Error: line_hint {line_hint} is ambiguous; " f"old_text appears {count} times." ) @@ -910,7 +910,7 @@ class EditFileTool(_FsTool): "or set replace_all=true." ) elif occurrence is not None and occurrence > count: - return ( + return ToolResult.error( f"Error: occurrence {occurrence} is out of range; " f"old_text appears {count} time." ) @@ -928,7 +928,7 @@ class EditFileTool(_FsTool): else: selected = [matches[occurrence - 1 if occurrence else 0]] if expected_replacements is not None and len(selected) != expected_replacements: - return ( + return ToolResult.error( f"Error: expected {expected_replacements} replacements but " f"would make {len(selected)}." ) @@ -954,9 +954,9 @@ class EditFileTool(_FsTool): msg = f"{warning}\n{msg}" return msg except PermissionError as e: - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") except Exception as e: - return f"Error editing file: {e}" + return ToolResult.error(f"Error editing file: {e}") def _file_not_found_msg(self, path: str, fp: Path) -> str: """Build an error message with 'Did you mean ...?' suggestions.""" @@ -969,7 +969,7 @@ class EditFileTool(_FsTool): parts = [f"Error: File not found: {path}"] if suggestions: parts.append("Did you mean: " + ", ".join(suggestions) + "?") - return "\n".join(parts) + return ToolResult.error("\n".join(parts)) @staticmethod def _not_found_msg(old_text: str, content: str, path: str) -> str: @@ -985,18 +985,18 @@ class EditFileTool(_FsTool): hint_text = "" if hints: hint_text = "\nPossible cause: " + ", ".join(hints) + "." - return ( + return ToolResult.error( f"Error: old_text not found in {path}." f"{hint_text}\nBest match ({best_ratio:.0%} similar) at line {best_start + 1}:\n{diff}" ) if hints: - return ( + return ToolResult.error( f"Error: old_text not found in {path}. " f"Possible cause: {', '.join(hints)}. " "Copy the exact text from read_file and try again." ) - return f"Error: old_text not found in {path}. No similar text found. Verify the file content." + return ToolResult.error(f"Error: old_text not found in {path}. No similar text found. Verify the file content.") # --------------------------------------------------------------------------- @@ -1051,9 +1051,9 @@ class ListDirTool(_FsTool): raise ValueError("Unknown path") dp = self._resolve(path) if not dp.exists(): - return f"Error: Directory not found: {path}" + return ToolResult.error(f"Error: Directory not found: {path}") if not dp.is_dir(): - return f"Error: Not a directory: {path}" + return ToolResult.error(f"Error: Not a directory: {path}") cap = max_entries or self._DEFAULT_MAX items: list[str] = [] @@ -1084,6 +1084,6 @@ class ListDirTool(_FsTool): result += f"\n\n(truncated, showing first {cap} of {total} entries)" return result except PermissionError as e: - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") except Exception as e: - return f"Error listing directory: {e}" + return ToolResult.error(f"Error listing directory: {e}") diff --git a/nanobot/agent/tools/image_generation.py b/nanobot/agent/tools/image_generation.py index 481d91f2..952a9352 100644 --- a/nanobot/agent/tools/image_generation.py +++ b/nanobot/agent/tools/image_generation.py @@ -7,7 +7,7 @@ from typing import TYPE_CHECKING, Any from pydantic import Field -from nanobot.agent.tools.base import Tool, tool_parameters +from nanobot.agent.tools.base import Tool, ToolResult, tool_parameters from nanobot.agent.tools.schema import ( ArraySchema, IntegerSchema, @@ -172,11 +172,11 @@ class ImageGenerationTool(Tool): ) -> str: client = self._provider_client() if client is None: - return f"Error: unsupported image generation provider '{self.config.provider}'" + return ToolResult.error(f"Error: unsupported image generation provider '{self.config.provider}'") requested = count or 1 if requested > self.config.max_images_per_turn: - return ( + return ToolResult.error( "Error: count exceeds tools.imageGeneration.maxImagesPerTurn " f"({self.config.max_images_per_turn})" ) @@ -206,4 +206,4 @@ class ImageGenerationTool(Tool): break return generated_image_tool_result(artifacts) except (ArtifactError, ImageGenerationError, OSError) as exc: - return f"Error: {exc}" + return ToolResult.error(f"Error: {exc}") diff --git a/nanobot/agent/tools/long_task.py b/nanobot/agent/tools/long_task.py index 12fcec17..887222f6 100644 --- a/nanobot/agent/tools/long_task.py +++ b/nanobot/agent/tools/long_task.py @@ -20,7 +20,7 @@ from contextvars import ContextVar from datetime import datetime from typing import TYPE_CHECKING, Any -from nanobot.agent.tools.base import Tool, tool_parameters +from nanobot.agent.tools.base import Tool, ToolResult, tool_parameters from nanobot.agent.tools.context import ContextAware, RequestContext from nanobot.agent.tools.schema import StringSchema, tool_parameters_schema from nanobot.bus.runtime_events import GoalStateChanged, RuntimeEventBus, RuntimeEventContext @@ -150,12 +150,12 @@ class LongTaskTool(Tool, _GoalToolsMixin): async def execute(self, goal: str, ui_summary: str | None = None, **kwargs: Any) -> str: sess = self._session() if sess is None: - return ( + return ToolResult.error( "Error: long_task requires an active chat session (missing routing context)." ) prior = parse_goal_state(goal_state_raw(sess.metadata)) if isinstance(prior, dict) and prior.get("status") == "active": - return ( + return ToolResult.error( "Error: a sustained goal is already active. " "Use complete_goal when finished, or ask the user before replacing it." ) @@ -230,7 +230,7 @@ class CompleteGoalTool(Tool, _GoalToolsMixin): async def execute(self, recap: str | None = None, **kwargs: Any) -> str: sess = self._session() if sess is None: - return "Error: complete_goal requires an active chat session." + return ToolResult.error("Error: complete_goal requires an active chat session.") prior = parse_goal_state(goal_state_raw(sess.metadata)) if not isinstance(prior, dict) or prior.get("status") != "active": return "No active goal to complete." diff --git a/nanobot/agent/tools/message.py b/nanobot/agent/tools/message.py index d7d5052e..2a009b22 100644 --- a/nanobot/agent/tools/message.py +++ b/nanobot/agent/tools/message.py @@ -6,7 +6,7 @@ from typing import Any, Awaitable, Callable from loguru import logger -from nanobot.agent.tools.base import Tool, tool_parameters +from nanobot.agent.tools.base import Tool, ToolResult, tool_parameters from nanobot.agent.tools.context import ContextAware, RequestContext from nanobot.agent.tools.path_utils import resolve_workspace_path from nanobot.agent.tools.schema import ArraySchema, StringSchema, tool_parameters_schema @@ -198,7 +198,7 @@ class MessageTool(Tool, ContextAware): not isinstance(row, list) or any(not isinstance(label, str) for label in row) for row in buttons ): - return "Error: buttons must be a list of list of strings" + return ToolResult.error("Error: buttons must be a list of list of strings") default_channel = self._default_channel.get() default_chat_id = self._default_chat_id.get() channel = channel or default_channel @@ -210,7 +210,7 @@ class MessageTool(Tool, ContextAware): and str(explicit_chat_id).strip() != "" and str(explicit_chat_id).strip() != str(default_chat_id).strip() ): - return ( + return ToolResult.error( "Error: chat_id does not match the active WebSocket conversation. " "Omit chat_id (and usually channel) so delivery uses the current " "conversation id from context — WebSocket client_id strings " @@ -229,16 +229,16 @@ class MessageTool(Tool, ContextAware): message_id = None if not channel or not chat_id: - return "Error: No target channel/chat specified" + return ToolResult.error("Error: No target channel/chat specified") if not self._send_callback: - return "Error: Message sending not configured" + return ToolResult.error("Error: Message sending not configured") if media: try: media = self._resolve_media(media) except (OSError, PermissionError, ValueError) as e: - return f"Error: media path is not allowed: {str(e)}" + return ToolResult.error(f"Error: media path is not allowed: {str(e)}") metadata = dict(self._default_metadata.get()) if same_target else {} if message_id: @@ -270,4 +270,4 @@ class MessageTool(Tool, ContextAware): button_info = f" with {sum(len(row) for row in buttons)} button(s)" if buttons else "" return f"Message sent to {channel}:{chat_id}{media_info}{button_info}" except Exception as e: - return f"Error sending message: {str(e)}" + return ToolResult.error(f"Error sending message: {str(e)}") diff --git a/nanobot/agent/tools/registry.py b/nanobot/agent/tools/registry.py index c60697ad..4cb608b4 100644 --- a/nanobot/agent/tools/registry.py +++ b/nanobot/agent/tools/registry.py @@ -3,7 +3,11 @@ import json from typing import Any -from nanobot.agent.tools.base import Tool +from nanobot.agent.tools.base import Tool, ToolResult + + +def is_tool_error_result(name: str, result: Any) -> bool: + return isinstance(result, ToolResult) and result.is_error class ToolRegistry: @@ -100,22 +104,26 @@ class ToolRegistry: suggestion = self._suggest_name(str(name)) hint = f" Did you mean '{suggestion}'? Tool names must match exactly." if suggestion else "" return None, params, ( - f"Error: Tool '{name}' not found.{hint} Available: {', '.join(self.tool_names)}" + ToolResult.error( + f"Error: Tool '{name}' not found.{hint} Available: {', '.join(self.tool_names)}" + ) ) params = self._coerce_params(tool, params) if not isinstance(params, dict): return tool, params, ( - f"Error: Tool '{name}' parameters must be a JSON object, got " - f"{type(params).__name__}. Use named parameters like " - 'tool_name(param1="value1", param2="value2") matching the tool schema.' + ToolResult.error( + f"Error: Tool '{name}' parameters must be a JSON object, got " + f"{type(params).__name__}. Use named parameters like " + 'tool_name(param1="value1", param2="value2") matching the tool schema.' + ) ) cast_params = tool.cast_params(params) errors = tool.validate_params(cast_params) if errors: return tool, cast_params, ( - f"Error: Invalid parameters for tool '{name}': " + "; ".join(errors) + ToolResult.error(f"Error: Invalid parameters for tool '{name}': " + "; ".join(errors)) ) return tool, cast_params, None @@ -159,16 +167,16 @@ class ToolRegistry: hint = "\n\n[Analyze the error above and try a different approach.]" tool, params, error = self.prepare_call(name, params) if error: - return error + hint + return ToolResult.error(str(error) + hint) try: assert tool is not None # guarded by prepare_call() result = await tool.execute(**params) - if isinstance(result, str) and result.startswith("Error"): - return result + hint + if is_tool_error_result(name, result): + return ToolResult.error(str(result) + hint) return result except Exception as e: - return f"Error executing {name}: {str(e)}" + hint + return ToolResult.error(f"Error executing {name}: {str(e)}" + hint) @property def tool_names(self) -> list[str]: diff --git a/nanobot/agent/tools/search.py b/nanobot/agent/tools/search.py index 30fefbb9..718d8e6d 100644 --- a/nanobot/agent/tools/search.py +++ b/nanobot/agent/tools/search.py @@ -9,6 +9,7 @@ from contextlib import suppress from pathlib import Path, PurePosixPath from typing import Any, Iterable, TypeVar +from nanobot.agent.tools.base import ToolResult from nanobot.agent.tools.filesystem import ListDirTool, _FsTool _DEFAULT_HEAD_LIMIT = 250 @@ -218,12 +219,12 @@ class FindFilesTool(_SearchTool): try: target = self._resolve(path or ".") if not target.exists(): - return f"Error: Path not found: {path}" + return ToolResult.error(f"Error: Path not found: {path}") if not (target.is_dir() or target.is_file()): - return f"Error: Unsupported path: {path}" + return ToolResult.error(f"Error: Unsupported path: {path}") if sort not in {"path", "modified"}: - return "Error: sort must be 'path' or 'modified'" + return ToolResult.error("Error: sort must be 'path' or 'modified'") limit = ( _DEFAULT_FILE_HEAD_LIMIT @@ -271,9 +272,9 @@ class FindFilesTool(_SearchTool): result += "\n\n" + note return result except PermissionError as e: - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") except Exception as e: - return f"Error finding files: {e}" + return ToolResult.error(f"Error finding files: {e}") class GrepTool(_SearchTool): @@ -425,16 +426,16 @@ class GrepTool(_SearchTool): try: target = self._resolve(path or ".") if not target.exists(): - return f"Error: Path not found: {path}" + return ToolResult.error(f"Error: Path not found: {path}") if not (target.is_dir() or target.is_file()): - return f"Error: Unsupported path: {path}" + return ToolResult.error(f"Error: Unsupported path: {path}") flags = re.IGNORECASE if case_insensitive else 0 try: needle = re.escape(pattern) if fixed_strings else pattern regex = re.compile(needle, flags) except re.error as e: - return f"Error: invalid regex pattern: {e}" + return ToolResult.error(f"Error: invalid regex pattern: {e}") if head_limit is not None: limit = None if head_limit == 0 else head_limit @@ -579,6 +580,6 @@ class GrepTool(_SearchTool): result += "\n\n" + "\n".join(notes) return result except PermissionError as e: - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") except Exception as e: - return f"Error searching files: {e}" + return ToolResult.error(f"Error searching files: {e}") diff --git a/nanobot/agent/tools/self.py b/nanobot/agent/tools/self.py index 9a5b82c0..48954272 100644 --- a/nanobot/agent/tools/self.py +++ b/nanobot/agent/tools/self.py @@ -7,7 +7,7 @@ from typing import TYPE_CHECKING, Any from loguru import logger -from nanobot.agent.tools.base import Tool +from nanobot.agent.tools.base import Tool, ToolResult from nanobot.agent.tools.context import ContextAware, RequestContext from nanobot.agent.tools.runtime_state import RuntimeState from nanobot.config_base import Base @@ -216,7 +216,7 @@ class MyTool(Tool, ContextAware): @staticmethod def _validate_key(key: str | None, label: str = "key") -> str | None: if not key or not key.strip(): - return f"Error: '{label}' cannot be empty or whitespace" + return ToolResult.error(f"Error: '{label}' cannot be empty or whitespace") return None # ------------------------------------------------------------------ @@ -321,7 +321,7 @@ class MyTool(Tool, ContextAware): if action in ("inspect", "check"): return self._inspect(key) if not self._modify_allowed: - return "Error: set is disabled (tools.my.allow_set is false)" + return ToolResult.error("Error: set is disabled (tools.my.allow_set is false)") if action in ("modify", "set"): return self._modify(key, value) return f"Unknown action: {action}" @@ -333,7 +333,7 @@ class MyTool(Tool, ContextAware): return self._inspect_all() top = key.split(".")[0] if top in self._DENIED_ATTRS or top.startswith("__"): - return f"Error: '{top}' is not accessible" + return ToolResult.error(f"Error: '{top}' is not accessible") obj, err = self._resolve_path(key) if err: # "scratchpad" alias for _runtime_vars @@ -343,12 +343,12 @@ class MyTool(Tool, ContextAware): # Fallback: check _runtime_vars for simple keys stored by modify if "." not in key and key in self._runtime_state._runtime_vars: return self._format_value(self._runtime_state._runtime_vars[key], key) - return f"Error: {err}" + return ToolResult.error(f"Error: {err}") # Guard against mock auto-generated attributes if "." not in key and not _has_real_attr(self._runtime_state, key): if key in self._runtime_state._runtime_vars: return self._format_value(self._runtime_state._runtime_vars[key], key) - return f"Error: '{key}' not found" + return ToolResult.error(f"Error: '{key}' not found") return self._format_value(obj, key) def _inspect_all(self) -> str: @@ -379,21 +379,21 @@ class MyTool(Tool, ContextAware): top = key.split(".")[0] if top in self.BLOCKED or top in self._DENIED_ATTRS or top.startswith("__") or top.lower() in self._SENSITIVE_NAMES: self._audit("modify", f"BLOCKED {key}") - return f"Error: '{key}' is protected and cannot be modified" + return ToolResult.error(f"Error: '{key}' is protected and cannot be modified") if top in self.READ_ONLY: self._audit("modify", f"READ_ONLY {key}") - return f"Error: '{key}' is read-only and cannot be modified" + return ToolResult.error(f"Error: '{key}' is read-only and cannot be modified") if "." in key: parent_path, leaf = key.rsplit(".", 1) if leaf in self._DENIED_ATTRS or leaf.startswith("__"): self._audit("modify", f"BLOCKED leaf '{leaf}'") - return f"Error: '{leaf}' is not accessible" + return ToolResult.error(f"Error: '{leaf}' is not accessible") if leaf.lower() in self._SENSITIVE_NAMES: self._audit("modify", f"BLOCKED sensitive leaf '{leaf}'") - return f"Error: '{leaf}' is not accessible" + return ToolResult.error(f"Error: '{leaf}' is not accessible") parent, err = self._resolve_path(parent_path) if err: - return f"Error: {err}" + return ToolResult.error(f"Error: {err}") if isinstance(parent, dict): parent[leaf] = value else: @@ -408,11 +408,11 @@ class MyTool(Tool, ContextAware): def _modify_model_preset(self, value: Any) -> str: if not isinstance(value, str) or not value.strip(): - return "Error: 'model_preset' must be a non-empty string" + return ToolResult.error("Error: 'model_preset' must be a non-empty string") name = value.strip() result = self._modify_free("model_preset", name) - if result.startswith("Error:"): - return result if result.endswith((".", "!", "?")) else f"{result}." + if isinstance(result, ToolResult) and result.is_error: + return result if result.endswith((".", "!", "?")) else ToolResult.error(f"{result}.") return ( f"{result}; model is now {self._runtime_state.model!r}; " f"context_window_tokens is now {self._runtime_state.context_window_tokens!r}" @@ -422,19 +422,19 @@ class MyTool(Tool, ContextAware): spec = self.RESTRICTED[key] expected = spec["type"] if expected is int and isinstance(value, bool): - return f"Error: '{key}' must be {expected.__name__}, got bool" + return ToolResult.error(f"Error: '{key}' must be {expected.__name__}, got bool") if not isinstance(value, expected): try: value = expected(value) except (ValueError, TypeError): - return f"Error: '{key}' must be {expected.__name__}, got {type(value).__name__}" + return ToolResult.error(f"Error: '{key}' must be {expected.__name__}, got {type(value).__name__}") old = getattr(self._runtime_state, key) if "min" in spec and value < spec["min"]: - return f"Error: '{key}' must be >= {spec['min']}" + return ToolResult.error(f"Error: '{key}' must be >= {spec['min']}") if "max" in spec and value > spec["max"]: - return f"Error: '{key}' must be <= {spec['max']}" + return ToolResult.error(f"Error: '{key}' must be <= {spec['max']}") if "min_len" in spec and len(str(value)) < spec["min_len"]: - return f"Error: '{key}' must be at least {spec['min_len']} characters" + return ToolResult.error(f"Error: '{key}' must be at least {spec['min_len']} characters") setattr(self._runtime_state, key, value) if key == "model": self._runtime_state._active_preset = None @@ -458,25 +458,25 @@ class MyTool(Tool, ContextAware): "modify", f"REJECTED type mismatch {key}: expects {old_t.__name__}, got {new_t.__name__}", ) - return f"Error: '{key}' expects {old_t.__name__}, got {new_t.__name__}" + return ToolResult.error(f"Error: '{key}' expects {old_t.__name__}, got {new_t.__name__}") try: setattr(self._runtime_state, key, value) except (ValueError, KeyError) as e: message = str(e.args[0] if isinstance(e, KeyError) and e.args else e).strip('"') self._audit("modify", f"REJECTED {key}: {message}") - return f"Error: {message}" + return ToolResult.error(f"Error: {message}") self._audit("modify", f"{key}: {old!r} -> {value!r}") return f"Set {key} = {value!r} (was {old!r})" if callable(value): self._audit("modify", f"REJECTED callable {key}") - return "Error: cannot store callable values" + return ToolResult.error("Error: cannot store callable values") err = self._validate_json_safe(value) if err: self._audit("modify", f"REJECTED {key}: {err}") - return f"Error: {err}" + return ToolResult.error(f"Error: {err}") if key not in self._runtime_state._runtime_vars and len(self._runtime_state._runtime_vars) >= self._MAX_RUNTIME_KEYS: self._audit("modify", f"REJECTED {key}: max keys ({self._MAX_RUNTIME_KEYS}) reached") - return f"Error: scratchpad is full (max {self._MAX_RUNTIME_KEYS} keys). Remove unused keys first." + return ToolResult.error(f"Error: scratchpad is full (max {self._MAX_RUNTIME_KEYS} keys). Remove unused keys first.") old = self._runtime_state._runtime_vars.get(key) self._runtime_state._runtime_vars[key] = value self._audit("modify", f"scratchpad.{key}: {old!r} -> {value!r}") diff --git a/nanobot/agent/tools/shell.py b/nanobot/agent/tools/shell.py index 28e2d0bf..dda5e15b 100644 --- a/nanobot/agent/tools/shell.py +++ b/nanobot/agent/tools/shell.py @@ -15,7 +15,7 @@ from typing import Any from loguru import logger from pydantic import Field -from nanobot.agent.tools.base import Tool, tool_parameters +from nanobot.agent.tools.base import Tool, ToolResult, tool_parameters from nanobot.agent.tools.context import current_request_session_key from nanobot.agent.tools.exec_session import ( DEFAULT_EXEC_SESSION_MANAGER, @@ -256,7 +256,7 @@ class ExecTool(Tool): command = command or cmd working_dir = working_dir or workdir if not command: - return "Error: Missing command. Provide command or cmd." + return ToolResult.error("Error: Missing command. Provide command or cmd.") if max_output_chars is None: max_output_chars = max_output_tokens @@ -283,7 +283,7 @@ class ExecTool(Tool): ) except asyncio.TimeoutError: await self._kill_process(process) - return f"Error: Command timed out after {prepared.timeout} seconds" + return ToolResult.error(f"Error: Command timed out after {prepared.timeout} seconds") except asyncio.CancelledError: await self._kill_process(process) raise @@ -314,7 +314,7 @@ class ExecTool(Tool): return result except Exception as e: - return f"Error executing command: {str(e)}" + return ToolResult.error(f"Error executing command: {str(e)}") async def _execute_session( self, @@ -339,9 +339,10 @@ class ExecTool(Tool): MAX_OUTPUT_CHARS, ), ) - return format_session_poll(session_id, poll) + result = format_session_poll(session_id, poll) + return ToolResult.error(result) if poll.timed_out else result except Exception as exc: - return f"Error executing command: {exc}" + return ToolResult.error(f"Error executing command: {exc}") def _resolve_timeout(self, timeout: int | None) -> int | None: """Resolve the effective hard timeout in seconds (None = no limit). @@ -383,12 +384,12 @@ class ExecTool(Tool): requested = Path(cwd).expanduser().resolve() resolved_root = Path(workspace_root).expanduser().resolve() except Exception: - return ( + return ToolResult.error( "Error: working_dir could not be resolved" + _WORKSPACE_BOUNDARY_NOTE ) if not is_path_within(requested, resolved_root): - return ( + return ToolResult.error( "Error: working_dir is outside the configured workspace" + _WORKSPACE_BOUNDARY_NOTE ) @@ -504,24 +505,24 @@ class ExecTool(Tool): if not shell: return None, None if _IS_WINDOWS: - return None, "Error: shell parameter is not supported on Windows" + return None, ToolResult.error("Error: shell parameter is not supported on Windows") if "\0" in shell or "\n" in shell or "\r" in shell: - return None, "Error: shell contains invalid characters" + return None, ToolResult.error("Error: shell contains invalid characters") allowed = {"sh", "bash", "zsh"} path = Path(shell).expanduser() if path.is_absolute(): if path.name not in allowed: - return None, f"Error: unsupported shell {shell!r}. Allowed: bash, sh, zsh" + return None, ToolResult.error(f"Error: unsupported shell {shell!r}. Allowed: bash, sh, zsh") if not path.is_file() or not os.access(path, os.X_OK): - return None, f"Error: shell is not executable: {shell}" + return None, ToolResult.error(f"Error: shell is not executable: {shell}") return str(path), None if "/" in shell or "\\" in shell: - return None, "Error: shell must be a shell name or absolute path" + return None, ToolResult.error("Error: shell must be a shell name or absolute path") if shell not in allowed: - return None, f"Error: unsupported shell {shell!r}. Allowed: bash, sh, zsh" + return None, ToolResult.error(f"Error: unsupported shell {shell!r}. Allowed: bash, sh, zsh") resolved = shutil.which(shell) if not resolved: - return None, f"Error: shell not found: {shell}" + return None, ToolResult.error(f"Error: shell not found: {shell}") return resolved, None @staticmethod @@ -608,10 +609,10 @@ class ExecTool(Tool): if not explicitly_allowed: for pattern in self.deny_patterns: if re.search(pattern, lower): - return "Error: Command blocked by deny pattern filter" + return ToolResult.error("Error: Command blocked by deny pattern filter") if self.allow_patterns: - return "Error: Command blocked by allowlist filter (not in allowlist)" + return ToolResult.error("Error: Command blocked by allowlist filter (not in allowlist)") from nanobot.security.network import contains_internal_url if contains_internal_url( @@ -621,12 +622,12 @@ class ExecTool(Tool): ), ): # The runner turns this marker into a non-retryable security hint. - return "Error: Command blocked by safety guard (internal/private URL detected)" + return ToolResult.error("Error: Command blocked by safety guard (internal/private URL detected)") should_restrict = self.restrict_to_workspace if restrict_to_workspace is None else restrict_to_workspace if should_restrict: if "..\\" in cmd or "../" in cmd: - return ( + return ToolResult.error( "Error: Command blocked by safety guard (path traversal detected)" + _WORKSPACE_BOUNDARY_NOTE ) @@ -661,7 +662,7 @@ class ExecTool(Tool): if not allowed and resolved_workspace is not None: allowed = is_path_within(p, resolved_workspace) if p.is_absolute() and not allowed: - return ( + return ToolResult.error( "Error: Command blocked by safety guard (path outside working dir)" + _WORKSPACE_BOUNDARY_NOTE ) diff --git a/nanobot/agent/tools/web.py b/nanobot/agent/tools/web.py index 947dc211..b919117a 100644 --- a/nanobot/agent/tools/web.py +++ b/nanobot/agent/tools/web.py @@ -14,7 +14,7 @@ import httpx from loguru import logger from pydantic import Field -from nanobot.agent.tools.base import Tool, tool_parameters +from nanobot.agent.tools.base import Tool, ToolResult, tool_parameters from nanobot.agent.tools.schema import ( BooleanSchema, IntegerSchema, @@ -395,13 +395,13 @@ class WebSearchTool(Tool): elif provider == "keenable": return await self._search_keenable(query, n) else: - return f"Error: unknown search provider '{provider}'" + return ToolResult.error(f"Error: unknown search provider '{provider}'") async def _search_olostep(self, query: str, n: int) -> str: try: from olostep import AsyncOlostep, Olostep_BaseError except ImportError: - return "Error: olostep package not installed. Run: pip install olostep" + return ToolResult.error("Error: olostep package not installed. Run: pip install olostep") api_key = self.config.api_key or os.environ.get("OLOSTEP_API_KEY", "") if not api_key: logger.warning("OLOSTEP_API_KEY not set, falling back to DuckDuckGo") @@ -481,13 +481,13 @@ class WebSearchTool(Tool): return _format_results(query, items, n) except httpx.HTTPStatusError as e: if e.response.status_code == 429: - return ( + return ToolResult.error( "Error: Brave search rate limited after retry. " "Retry later or reduce consecutive web_search calls." ) - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") except Exception as e: - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") async def _search_tavily(self, query: str, n: int) -> str: api_key = self.config.api_key or os.environ.get("TAVILY_API_KEY", "") @@ -505,7 +505,7 @@ class WebSearchTool(Tool): r.raise_for_status() return _format_results(query, r.json().get("results", []), n) except Exception as e: - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") async def _search_keenable(self, query: str, n: int) -> str: api_key = self.config.api_key or os.environ.get("KEENABLE_API_KEY", "") @@ -540,10 +540,10 @@ class WebSearchTool(Tool): return _format_results(query, items, n) except httpx.HTTPStatusError as e: if e.response.status_code == 429: - return "Error: Keenable search rate limited. Try again later or reduce search frequency." - return f"Error: Keenable search failed ({e.response.status_code}): {e}" + return ToolResult.error("Error: Keenable search rate limited. Try again later or reduce search frequency.") + return ToolResult.error(f"Error: Keenable search failed ({e.response.status_code}): {e}") except Exception as e: - return f"Error: Keenable search failed: {e}" + return ToolResult.error(f"Error: Keenable search failed: {e}") async def _search_searxng(self, query: str, n: int) -> str: base_url = (self.config.base_url or os.environ.get("SEARXNG_BASE_URL", "")).strip() @@ -553,7 +553,7 @@ class WebSearchTool(Tool): endpoint = f"{base_url.rstrip('/')}/search" is_valid, error_msg = _validate_url(endpoint) if not is_valid: - return f"Error: invalid SearXNG URL: {error_msg}" + return ToolResult.error(f"Error: invalid SearXNG URL: {error_msg}") try: async with httpx.AsyncClient(proxy=self.proxy) as client: r = await client.get( @@ -565,7 +565,7 @@ class WebSearchTool(Tool): r.raise_for_status() return _format_results(query, r.json().get("results", []), n) except Exception as e: - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") async def _search_jina(self, query: str, n: int) -> str: api_key = self.config.api_key or os.environ.get("JINA_API_KEY", "") @@ -616,7 +616,7 @@ class WebSearchTool(Tool): ] return _format_results(query, items, n) except Exception as e: - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") async def _search_exa(self, query: str, n: int) -> str: api_key = self.config.api_key or os.environ.get("EXA_API_KEY", "") @@ -663,10 +663,10 @@ class WebSearchTool(Tool): return _format_results(query, items, n) except httpx.HTTPStatusError as e: if e.response.status_code == 429: - return "Error: Exa search rate limited. Try again later or reduce search frequency." - return f"Error: Exa search failed ({e.response.status_code}): {e}" + return ToolResult.error("Error: Exa search rate limited. Try again later or reduce search frequency.") + return ToolResult.error(f"Error: Exa search failed ({e.response.status_code}): {e}") except Exception as e: - return f"Error: Exa search failed: {e}" + return ToolResult.error(f"Error: Exa search failed: {e}") async def _search_volcengine( self, @@ -690,7 +690,7 @@ class WebSearchTool(Tool): normalized_time_range = _normalize_volcengine_time_range(time_range) if time_range else None normalized_auth_level = _normalize_volcengine_auth_level(auth_level) if auth_level is not None else None except ValueError as e: - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") body: dict[str, Any] = { "Query": query, @@ -723,18 +723,18 @@ class WebSearchTool(Tool): data = r.json() except httpx.HTTPStatusError as e: if e.response.status_code == 429: - return "Error: Volcengine search rate limited. Try again later or reduce search frequency." - return f"Error: Volcengine search failed ({e.response.status_code}): {e}" + return ToolResult.error("Error: Volcengine search rate limited. Try again later or reduce search frequency.") + return ToolResult.error(f"Error: Volcengine search failed ({e.response.status_code}): {e}") except Exception as e: - return f"Error: Volcengine search failed: {e}" + return ToolResult.error(f"Error: Volcengine search failed: {e}") error = (data.get("ResponseMetadata") or {}).get("Error") or data.get("Error") or data.get("error") if error: if isinstance(error, dict): code = error.get("Code") or error.get("code") or "unknown" message = error.get("Message") or error.get("message") or error - return f"Error: Volcengine search error {code}: {message}" - return f"Error: Volcengine search error: {error}" + return ToolResult.error(f"Error: Volcengine search error {code}: {message}") + return ToolResult.error(f"Error: Volcengine search error: {error}") result = data.get("Result") or data web_results = result.get("WebResults") or result.get("webResults") or result.get("results") or [] @@ -791,7 +791,7 @@ class WebSearchTool(Tool): return _format_results(query, items, n) except Exception as e: logger.warning("DuckDuckGo search failed: {}", e) - return f"Error: DuckDuckGo search failed ({e})" + return ToolResult.error(f"Error: DuckDuckGo search failed ({e})") async def _search_bocha(self, query: str, n: int, freshness: str = "noLimit") -> str: api_key = self.config.api_key or os.environ.get("BOCHA_API_KEY", "") @@ -819,7 +819,7 @@ class WebSearchTool(Tool): timeout=self.config.timeout, ) if r.status_code == 429: - return "Error: Bocha search rate-limited (HTTP 429). Wait and retry." + return ToolResult.error("Error: Bocha search rate-limited (HTTP 429). Wait and retry.") r.raise_for_status() data = r.json() wrapped_data = data.get("data") if isinstance(data, dict) else None @@ -839,9 +839,9 @@ class WebSearchTool(Tool): ] return _format_results(query, items, n) except httpx.HTTPStatusError as e: - return f"Error: Bocha search HTTP {e.response.status_code}: {e.response.text[:200]}" + return ToolResult.error(f"Error: Bocha search HTTP {e.response.status_code}: {e.response.text[:200]}") except Exception as e: - return f"Error: {e}" + return ToolResult.error(f"Error: {e}") @tool_parameters( diff --git a/tests/agent/test_runner_errors.py b/tests/agent/test_runner_errors.py index 65550377..5d05ae0f 100644 --- a/tests/agent/test_runner_errors.py +++ b/tests/agent/test_runner_errors.py @@ -135,6 +135,46 @@ async def test_runner_tool_error_sets_final_content(): assert result.stop_reason == "tool_error" +@pytest.mark.asyncio +async def test_runner_preserves_successful_exec_output_that_starts_with_error(): + from nanobot.agent.runner import AgentRunSpec, AgentRunner + + provider = MagicMock(spec=LLMProvider) + + async def chat_with_retry(*, messages, **kwargs): + if not any(msg.get("role") == "tool" for msg in messages): + return LLMResponse( + content="working", + tool_calls=[ + ToolCallRequest(id="call_1", name="exec", arguments={"command": "report"}) + ], + usage={}, + ) + return LLMResponse(content="done", usage={}) + + provider.chat_with_retry = chat_with_retry + output = "Error: generated report successfully\n\nExit code: 0" + tools = MagicMock() + tools.get_definitions.return_value = [] + tools.execute = AsyncMock(return_value=output) + + runner = AgentRunner(provider) + result = await runner.run(AgentRunSpec( + initial_messages=[{"role": "user", "content": "run report"}], + tools=tools, + model="test-model", + max_iterations=2, + max_tool_result_chars=_MAX_TOOL_RESULT_CHARS, + fail_on_tool_error=True, + )) + + assert result.final_content == "done" + assert result.stop_reason == "completed" + assert result.tool_events == [ + {"name": "exec", "status": "ok", "detail": "Error: generated report successfully Exit code: 0"} + ] + + @pytest.mark.asyncio async def test_runner_tool_error_preserves_tool_results_in_messages(): """When a tool raises a fatal error, its results must still be appended diff --git a/tests/agent/test_runner_safety.py b/tests/agent/test_runner_safety.py index 14565e20..1fc467db 100644 --- a/tests/agent/test_runner_safety.py +++ b/tests/agent/test_runner_safety.py @@ -6,6 +6,8 @@ from unittest.mock import AsyncMock, MagicMock import pytest +from nanobot.agent.runner import AgentRunner, AgentRunSpec +from nanobot.agent.tools import ToolResult from nanobot.config.schema import AgentDefaults from nanobot.providers.base import LLMResponse, ToolCallRequest @@ -20,8 +22,6 @@ async def test_runner_does_not_abort_on_workspace_violation_anymore(): we now hand the error back to the LLM as a recoverable tool result and rely on ``repeated_workspace_violation_error`` to throttle bypass loops. """ - from nanobot.agent.runner import AgentRunSpec, AgentRunner - provider = MagicMock() provider.chat_with_retry = AsyncMock(side_effect=[ LLMResponse( @@ -64,8 +64,6 @@ async def test_runner_does_not_abort_on_workspace_violation_anymore(): def test_is_ssrf_violation_recognizes_private_url_blocks(): """SSRF rejections are classified separately from workspace boundaries.""" - from nanobot.agent.runner import AgentRunner - ssrf_msg = "Error: Command blocked by safety guard (internal/private URL detected)" assert AgentRunner._is_ssrf_violation(ssrf_msg) is True assert AgentRunner._is_ssrf_violation( @@ -88,8 +86,6 @@ def test_is_ssrf_violation_recognizes_private_url_blocks(): @pytest.mark.asyncio async def test_runner_returns_non_retryable_hint_on_ssrf_violation(): """SSRF stays blocked, but the runtime gives the LLM a final chance to recover.""" - from nanobot.agent.runner import AgentRunSpec, AgentRunner - provider = MagicMock() provider.chat_with_retry = AsyncMock(side_effect=[ LLMResponse( @@ -107,7 +103,7 @@ async def test_runner_returns_non_retryable_hint_on_ssrf_violation(): ]) tools = MagicMock() tools.get_definitions.return_value = [] - tools.execute = AsyncMock(return_value=( + tools.execute = AsyncMock(return_value=ToolResult.error( "Error: Command blocked by safety guard (internal/private URL detected)" )) @@ -141,8 +137,6 @@ async def test_runner_lets_llm_recover_from_shell_guard_path_outside(): turn (silent hang on Telegram per #3605); now the LLM gets the soft error back and can finalize on the next iteration. """ - from nanobot.agent.runner import AgentRunSpec, AgentRunner - provider = MagicMock() captured_second_call: list[dict] = [] @@ -163,7 +157,9 @@ async def test_runner_lets_llm_recover_from_shell_guard_path_outside(): tools = MagicMock() tools.get_definitions.return_value = [] tools.execute = AsyncMock( - return_value="Error: Command blocked by safety guard (path outside working dir)" + return_value=ToolResult.error( + "Error: Command blocked by safety guard (path outside working dir)" + ) ) runner = AgentRunner(provider) @@ -195,8 +191,6 @@ async def test_runner_throttles_repeated_workspace_bypass_attempts(): the runner replaces the tool result with a hard "stop trying" message so the model finally gives up and surfaces the boundary to the user. """ - from nanobot.agent.runner import AgentRunSpec, AgentRunner - bypass_attempts = [ ToolCallRequest( id=f"a{i}", name="exec", @@ -215,7 +209,9 @@ async def test_runner_throttles_repeated_workspace_bypass_attempts(): tools = MagicMock() tools.get_definitions.return_value = [] tools.execute = AsyncMock( - return_value="Error: Command blocked by safety guard (path outside working dir)" + return_value=ToolResult.error( + "Error: Command blocked by safety guard (path outside working dir)" + ) ) runner = AgentRunner(provider) diff --git a/tests/tools/test_exec_session_tools.py b/tests/tools/test_exec_session_tools.py index 3ef3f37b..e120b236 100644 --- a/tests/tools/test_exec_session_tools.py +++ b/tests/tools/test_exec_session_tools.py @@ -11,6 +11,7 @@ from nanobot.agent.tools.exec_session import ( ListExecSessionsTool, WriteStdinTool, ) +from nanobot.agent.tools.registry import is_tool_error_result from nanobot.agent.tools.shell import ExecTool @@ -334,9 +335,10 @@ def test_write_stdin_reports_missing_session(tmp_path): manager = ExecSessionManager() tool = WriteStdinTool(manager=manager) - result = asyncio.run(tool.execute(session_id="missing", chars="")) + result = asyncio.run(tool.execute(session_id="missing\nExit code: 0", chars="")) - assert "exec session not found" in result + assert result == "Error: exec session not found: 'missing\\nExit code: 0'" + assert is_tool_error_result("write_stdin", result) def test_list_exec_sessions_reports_running_commands(tmp_path): diff --git a/tests/tools/test_tool_registry.py b/tests/tools/test_tool_registry.py index 7d6bdaaf..ff784568 100644 --- a/tests/tools/test_tool_registry.py +++ b/tests/tools/test_tool_registry.py @@ -1,8 +1,9 @@ from __future__ import annotations from typing import Any +from unittest.mock import AsyncMock -from nanobot.agent.tools.base import Tool +from nanobot.agent.tools.base import Tool, ToolResult from nanobot.agent.tools.filesystem import ReadFileTool from nanobot.agent.tools.registry import ToolRegistry @@ -257,6 +258,41 @@ async def test_registry_rejects_unknown_builtin_tool_parameters(tmp_path) -> Non assert "one" not in result +async def test_registry_preserves_successful_exec_output_that_starts_with_error() -> None: + registry = ToolRegistry() + output = "Error: generated report successfully\n\nExit code: 0" + tool = _FakeTool("exec") + tool.execute = AsyncMock(return_value=output) + registry.register(tool) + + result = await registry.execute("exec", {}) + + assert result == output + + +async def test_registry_uses_structured_tool_result_for_errors() -> None: + registry = ToolRegistry() + output = "Error: plain tool output, not a structured failure" + raw_tool = _FakeTool("raw_output") + raw_tool.execute = AsyncMock(return_value=output) + registry.register(raw_tool) + + raw_result = await registry.execute("raw_output", {}) + + assert raw_result == output + + failing_tool = _FakeTool("failing_tool") + failing_tool.execute = AsyncMock(return_value=ToolResult.error("Error: real failure")) + registry.register(failing_tool) + + error_result = await registry.execute("failing_tool", {}) + + assert isinstance(error_result, ToolResult) + assert error_result.is_error + assert error_result.startswith("Error: real failure") + assert "[Analyze the error above" in error_result + + def test_get_definitions_returns_cached_result() -> None: registry = ToolRegistry() registry.register(_FakeTool("read_file"))