refactor(tools): use structured tool error results

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

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)

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):

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"))