mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-31 00:03:01 +03:00
fix(agent): let subagents recover from tool errors
This commit is contained in:
+1
-12
@@ -2225,22 +2225,11 @@ By default, nanobot only allows one spawned subagent at a time. When the limit i
|
||||
}
|
||||
```
|
||||
|
||||
Subagents also stop immediately when one of their tools returns an execution error. That default keeps failures visible to the parent agent. If your subagent workflows use tools that can fail transiently and should be retried or worked around by the model, disable hard-stop behavior:
|
||||
|
||||
```json
|
||||
{
|
||||
"agents": {
|
||||
"defaults": {
|
||||
"failOnToolError": false
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
Tool execution errors are returned to the subagent model so it can retry or choose a different approach within the same run. Provider failures, cancellation, and the maximum tool-iteration limit still stop the run.
|
||||
|
||||
| Option | Default | Description |
|
||||
|--------|---------|-------------|
|
||||
| `agents.defaults.maxConcurrentSubagents` | `1` | Maximum number of spawned subagents that may run at the same time. Attempts to spawn beyond this limit return an error. |
|
||||
| `agents.defaults.failOnToolError` | `true` | Stop a spawned subagent when a tool execution fails. Set to `false` to return tool errors to the subagent model so it can recover within the same run. |
|
||||
|
||||
|
||||
## Auto Compact
|
||||
|
||||
@@ -270,7 +270,6 @@ class AgentLoop:
|
||||
context_window_tokens: int | None = None,
|
||||
context_block_limit: int | None = None,
|
||||
max_tool_result_chars: int | None = None,
|
||||
fail_on_tool_error: bool | None = None,
|
||||
provider_retry_mode: str = "standard",
|
||||
tool_hint_max_length: int | None = None,
|
||||
cron_service: CronService | None = None,
|
||||
@@ -404,7 +403,6 @@ class AgentLoop:
|
||||
disabled_skills=disabled_skills,
|
||||
max_iterations=self.max_iterations,
|
||||
max_concurrent_subagents=max_concurrent_subagents,
|
||||
fail_on_tool_error=fail_on_tool_error,
|
||||
llm_wall_timeout_for_session=lambda sk: runner_wall_llm_timeout_s(self.sessions, sk),
|
||||
)
|
||||
self._unified_session = unified_session
|
||||
@@ -518,7 +516,6 @@ class AgentLoop:
|
||||
context_window_tokens=context_window_tokens,
|
||||
context_block_limit=defaults.context_block_limit,
|
||||
max_tool_result_chars=defaults.max_tool_result_chars,
|
||||
fail_on_tool_error=defaults.fail_on_tool_error,
|
||||
provider_retry_mode=defaults.provider_retry_mode,
|
||||
tool_hint_max_length=defaults.tool_hint_max_length,
|
||||
restrict_to_workspace=config.tools.restrict_to_workspace,
|
||||
|
||||
+16
-46
@@ -109,7 +109,6 @@ class AgentRunSpec:
|
||||
error_message: str | None = _DEFAULT_ERROR_MESSAGE
|
||||
max_iterations_message: str | None = None
|
||||
concurrent_tools: bool = False
|
||||
fail_on_tool_error: bool = False
|
||||
workspace: Path | None = None
|
||||
session_key: str | None = None
|
||||
context_block_limit: int | None = None
|
||||
@@ -570,7 +569,7 @@ class AgentRunner:
|
||||
|
||||
await hook.before_execute_tools(context)
|
||||
|
||||
results, new_events, fatal_error = await self._execute_tools(
|
||||
results, new_events = await self._execute_tools(
|
||||
spec,
|
||||
response.tool_calls,
|
||||
external_lookup_counts,
|
||||
@@ -601,24 +600,6 @@ class AgentRunner:
|
||||
}
|
||||
messages.append(tool_message)
|
||||
completed_tool_results.append(tool_message)
|
||||
if fatal_error is not None:
|
||||
error = f"Error: {type(fatal_error).__name__}: {fatal_error}"
|
||||
final_content = error
|
||||
stop_reason = "tool_error"
|
||||
self._append_final_message(messages, final_content)
|
||||
context.final_content = final_content
|
||||
context.error = error
|
||||
context.stop_reason = stop_reason
|
||||
await hook.after_iteration(context)
|
||||
should_continue, injection_cycles = await self._try_drain_injections(
|
||||
spec, messages, None, injection_cycles,
|
||||
phase="after tool error",
|
||||
)
|
||||
if should_continue:
|
||||
had_injections = True
|
||||
length_recovery_parts.clear()
|
||||
continue
|
||||
break
|
||||
checkpoint_model_messages = (
|
||||
self.context_governor.prepare_for_model(
|
||||
governance_config,
|
||||
@@ -1422,11 +1403,11 @@ class AgentRunner:
|
||||
workspace_violation_counts: dict[str, int],
|
||||
hook: AgentHook | None = None,
|
||||
context: AgentHookContext | None = None,
|
||||
) -> tuple[list[Any], list[dict[str, str]], BaseException | None]:
|
||||
) -> tuple[list[Any], list[dict[str, str]]]:
|
||||
hook = hook or AgentHook()
|
||||
context = context or AgentHookContext(iteration=0, messages=[])
|
||||
batches = self._partition_tool_batches(spec, tool_calls)
|
||||
tool_results: list[tuple[Any, dict[str, str], BaseException | None]] = []
|
||||
tool_results: list[tuple[Any, dict[str, str]]] = []
|
||||
for batch in batches:
|
||||
if spec.concurrent_tools and len(batch) > 1:
|
||||
batch_results = await asyncio.gather(*(
|
||||
@@ -1442,7 +1423,7 @@ class AgentRunner:
|
||||
))
|
||||
tool_results.extend(batch_results)
|
||||
else:
|
||||
batch_results: list[tuple[Any, dict[str, str], BaseException | None]] = []
|
||||
batch_results: list[tuple[Any, dict[str, str]]] = []
|
||||
for tool_call in batch:
|
||||
result = await self._run_tool(
|
||||
spec,
|
||||
@@ -1457,13 +1438,10 @@ class AgentRunner:
|
||||
|
||||
results: list[Any] = []
|
||||
events: list[dict[str, str]] = []
|
||||
fatal_error: BaseException | None = None
|
||||
for result, event, error in tool_results:
|
||||
for result, event in tool_results:
|
||||
results.append(result)
|
||||
events.append(event)
|
||||
if error is not None and fatal_error is None:
|
||||
fatal_error = error
|
||||
return results, events, fatal_error
|
||||
return results, events
|
||||
|
||||
async def _run_tool(
|
||||
self,
|
||||
@@ -1473,7 +1451,7 @@ class AgentRunner:
|
||||
workspace_violation_counts: dict[str, int],
|
||||
hook: AgentHook | None = None,
|
||||
context: AgentHookContext | None = None,
|
||||
) -> tuple[Any, dict[str, str], BaseException | None]:
|
||||
) -> tuple[Any, dict[str, str]]:
|
||||
hook = hook or AgentHook()
|
||||
context = context or AgentHookContext(iteration=0, messages=[])
|
||||
hint = "\n\n[Analyze the error above and try a different approach.]"
|
||||
@@ -1488,9 +1466,7 @@ class AgentRunner:
|
||||
"status": "error",
|
||||
"detail": "repeated external lookup blocked",
|
||||
}
|
||||
if spec.fail_on_tool_error:
|
||||
return lookup_error + hint, event, RuntimeError(lookup_error)
|
||||
return lookup_error + hint, event, None
|
||||
return lookup_error + hint, event
|
||||
prepare_call = cast(
|
||||
Callable[[str, Any], object] | None,
|
||||
getattr(spec.tools, "prepare_call", None),
|
||||
@@ -1517,9 +1493,7 @@ class AgentRunner:
|
||||
)
|
||||
if handled is not None:
|
||||
return handled
|
||||
return prep_error + hint, event, (
|
||||
RuntimeError(prep_error) if spec.fail_on_tool_error else None
|
||||
)
|
||||
return prep_error + hint, event
|
||||
await hook.before_execute_tool(context, tool_call, tool, params)
|
||||
try:
|
||||
if tool is not None:
|
||||
@@ -1546,9 +1520,7 @@ class AgentRunner:
|
||||
)
|
||||
if handled is not None:
|
||||
return handled
|
||||
if spec.fail_on_tool_error:
|
||||
return payload, event, exc
|
||||
return payload, event, None
|
||||
return payload, event
|
||||
|
||||
if is_tool_error_result(result):
|
||||
await hook.on_execute_tool_error(context, tool_call, tool, params, result)
|
||||
@@ -1566,9 +1538,7 @@ class AgentRunner:
|
||||
)
|
||||
if handled is not None:
|
||||
return handled
|
||||
if spec.fail_on_tool_error:
|
||||
return result + hint, event, RuntimeError(result)
|
||||
return result + hint, event, None
|
||||
return result + hint, event
|
||||
|
||||
await hook.after_execute_tool(context, tool_call, tool, params, result)
|
||||
|
||||
@@ -1578,7 +1548,7 @@ class AgentRunner:
|
||||
detail = "(empty)"
|
||||
elif len(detail) > 120:
|
||||
detail = detail[:120] + "..."
|
||||
return result, {"name": tool_call.name, "status": "ok", "detail": detail}, None
|
||||
return result, {"name": tool_call.name, "status": "ok", "detail": detail}
|
||||
|
||||
# SSRF is a hard security block at the tool boundary, but the agent turn
|
||||
# should recover conversationally instead of aborting the runtime.
|
||||
@@ -1631,7 +1601,7 @@ class AgentRunner:
|
||||
event: dict[str, str],
|
||||
tool_call: ToolCallRequest,
|
||||
workspace_violation_counts: dict[str, int],
|
||||
) -> tuple[Any, dict[str, str], BaseException | None] | None:
|
||||
) -> tuple[Any, dict[str, str]] | None:
|
||||
"""Classify safety-boundary failures, or return ``None`` to pass through."""
|
||||
if self._is_ssrf_violation(raw_text):
|
||||
logger.warning(
|
||||
@@ -1640,7 +1610,7 @@ class AgentRunner:
|
||||
raw_text.replace("\n", " ").strip()[:200],
|
||||
)
|
||||
event["detail"] = self._event_detail("ssrf_violation: ", raw_text)
|
||||
return self._ssrf_soft_payload(raw_text), event, None
|
||||
return self._ssrf_soft_payload(raw_text), event
|
||||
|
||||
if self._is_workspace_violation(raw_text):
|
||||
escalation = repeated_workspace_violation_error(
|
||||
@@ -1658,8 +1628,8 @@ class AgentRunner:
|
||||
"workspace_violation_escalated: ",
|
||||
raw_text,
|
||||
)
|
||||
return escalation, event, None
|
||||
return soft_payload, event, None
|
||||
return escalation, event
|
||||
return soft_payload, event
|
||||
|
||||
return None
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@ from typing import Any, Callable, NotRequired, TypedDict
|
||||
from loguru import logger
|
||||
|
||||
from nanobot.agent.hook import AgentHook, AgentHookContext
|
||||
from nanobot.agent.runner import AgentRunner, AgentRunResult, AgentRunSpec
|
||||
from nanobot.agent.runner import AgentRunner, AgentRunSpec
|
||||
from nanobot.agent.tools.base import ToolResult
|
||||
from nanobot.agent.tools.context import (
|
||||
RequestContext,
|
||||
@@ -104,7 +104,6 @@ class SubagentManager:
|
||||
disabled_skills: list[str] | None = None,
|
||||
max_iterations: int | None = None,
|
||||
max_concurrent_subagents: int | None = None,
|
||||
fail_on_tool_error: bool | None = None,
|
||||
llm_wall_timeout_for_session: Callable[[str | None], float | None] | None = None,
|
||||
):
|
||||
if workspace is None:
|
||||
@@ -148,11 +147,6 @@ class SubagentManager:
|
||||
if max_concurrent_subagents is not None
|
||||
else defaults.max_concurrent_subagents
|
||||
)
|
||||
self.fail_on_tool_error = (
|
||||
fail_on_tool_error
|
||||
if fail_on_tool_error is not None
|
||||
else defaults.fail_on_tool_error
|
||||
)
|
||||
self.runner = AgentRunner()
|
||||
self._exec_session_manager = ExecSessionManager()
|
||||
self._llm_wall_timeout_for_session = llm_wall_timeout_for_session
|
||||
@@ -346,7 +340,7 @@ class SubagentManager:
|
||||
self._session_tasks.setdefault(session_key, set()).add(task_id)
|
||||
try:
|
||||
result = await inline_task
|
||||
if status.phase == "error" or status.stop_reason in {"error", "tool_error"}:
|
||||
if status.phase == "error" or status.stop_reason == "error":
|
||||
return ToolResult.error(result)
|
||||
return result
|
||||
finally:
|
||||
@@ -416,7 +410,6 @@ class SubagentManager:
|
||||
max_iterations_message="Task completed but no final response was generated.",
|
||||
finalize_on_max_iterations=False,
|
||||
error_message=None,
|
||||
fail_on_tool_error=self.fail_on_tool_error,
|
||||
checkpoint_callback=_on_checkpoint,
|
||||
session_key=sess_key,
|
||||
workspace=root,
|
||||
@@ -433,11 +426,7 @@ class SubagentManager:
|
||||
status.phase = "done"
|
||||
status.stop_reason = result.stop_reason
|
||||
|
||||
if result.stop_reason == "tool_error":
|
||||
status.tool_events = list(result.tool_events)
|
||||
final_result = self._format_partial_progress(result)
|
||||
final_status = "error"
|
||||
elif result.stop_reason == "error":
|
||||
if result.stop_reason == "error":
|
||||
final_result = result.error or "Error: subagent execution failed."
|
||||
final_status = "error"
|
||||
else:
|
||||
@@ -518,27 +507,6 @@ class SubagentManager:
|
||||
await self.bus.publish_inbound(msg)
|
||||
logger.debug("Subagent [{}] announced result to {}:{}", task_id, origin['channel'], origin['chat_id'])
|
||||
|
||||
@staticmethod
|
||||
def _format_partial_progress(result: AgentRunResult) -> str:
|
||||
completed = [e for e in result.tool_events if e["status"] == "ok"]
|
||||
failure = next((e for e in reversed(result.tool_events) if e["status"] == "error"), None)
|
||||
lines: list[str] = []
|
||||
if completed:
|
||||
lines.append("Completed steps:")
|
||||
for event in completed[-3:]:
|
||||
lines.append(f"- {event['name']}: {event['detail']}")
|
||||
if failure:
|
||||
if lines:
|
||||
lines.append("")
|
||||
lines.append("Failure:")
|
||||
lines.append(f"- {failure['name']}: {failure['detail']}")
|
||||
if result.error and not failure:
|
||||
if lines:
|
||||
lines.append("")
|
||||
lines.append("Failure:")
|
||||
lines.append(f"- {result.error}")
|
||||
return "\n".join(lines) or (result.error or "Error: subagent execution failed.")
|
||||
|
||||
def _build_subagent_prompt(self, workspace: Path | None = None) -> str:
|
||||
"""Build a focused system prompt for the subagent."""
|
||||
from nanobot.agent.skills import SkillsLoader
|
||||
|
||||
@@ -129,7 +129,6 @@ class AgentDefaults(Base):
|
||||
fallback_models: list[FallbackCandidate] = Field(default_factory=list)
|
||||
max_tool_iterations: int = 200
|
||||
max_concurrent_subagents: int = Field(default=1, ge=1)
|
||||
fail_on_tool_error: bool = True
|
||||
max_tool_result_chars: int = 16_000
|
||||
provider_retry_mode: Literal["standard", "persistent"] = "standard"
|
||||
tool_hint_max_length: int = Field(
|
||||
|
||||
@@ -9,6 +9,7 @@ from unittest.mock import AsyncMock, MagicMock
|
||||
import pytest
|
||||
|
||||
from agent.runner_helpers import make_run_spec
|
||||
from nanobot.agent.tools import ToolResult
|
||||
from nanobot.config.schema import AgentDefaults
|
||||
from nanobot.providers.base import LLMProvider, LLMResponse, ToolCallRequest
|
||||
|
||||
@@ -16,14 +17,17 @@ _MAX_TOOL_RESULT_CHARS = AgentDefaults().max_tool_result_chars
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_returns_structured_tool_error():
|
||||
async def test_runner_returns_tool_exception_to_model_for_recovery():
|
||||
from nanobot.agent.runner import AgentRunner
|
||||
|
||||
provider = MagicMock(spec=LLMProvider)
|
||||
provider.chat_with_retry = AsyncMock(return_value=LLMResponse(
|
||||
content="working",
|
||||
tool_calls=[ToolCallRequest(id="call_1", name="list_dir", arguments={})],
|
||||
))
|
||||
provider.chat_with_retry = AsyncMock(side_effect=[
|
||||
LLMResponse(
|
||||
content="working",
|
||||
tool_calls=[ToolCallRequest(id="call_1", name="list_dir", arguments={})],
|
||||
),
|
||||
LLMResponse(content="recovered", tool_calls=[]),
|
||||
])
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
tools.execute = AsyncMock(side_effect=RuntimeError("boom"))
|
||||
@@ -36,14 +40,17 @@ async def test_runner_returns_structured_tool_error():
|
||||
model="test-model",
|
||||
max_iterations=2,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
fail_on_tool_error=True,
|
||||
))
|
||||
|
||||
assert result.stop_reason == "tool_error"
|
||||
assert result.error == "Error: RuntimeError: boom"
|
||||
assert provider.chat_with_retry.await_count == 2
|
||||
assert result.stop_reason == "completed"
|
||||
assert result.error is None
|
||||
assert result.final_content == "recovered"
|
||||
assert result.tool_events == [
|
||||
{"name": "list_dir", "status": "error", "detail": "boom"}
|
||||
]
|
||||
tool_message = next(message for message in result.messages if message.get("role") == "tool")
|
||||
assert "Error: RuntimeError: boom" in tool_message["content"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -180,35 +187,38 @@ async def test_runner_ignores_tool_calls_when_finish_reason_blocks_execution(
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_tool_error_sets_final_content():
|
||||
async def test_runner_returns_structured_tool_error_to_model_for_recovery():
|
||||
from nanobot.agent.runner import AgentRunner
|
||||
|
||||
provider = MagicMock(spec=LLMProvider)
|
||||
|
||||
async def chat_with_retry(*, messages, **kwargs):
|
||||
return LLMResponse(
|
||||
provider.chat_with_retry = AsyncMock(side_effect=[
|
||||
LLMResponse(
|
||||
content="working",
|
||||
tool_calls=[ToolCallRequest(id="call_1", name="read_file", arguments={"path": "x"})],
|
||||
usage=None,
|
||||
)
|
||||
|
||||
provider.chat_with_retry = chat_with_retry
|
||||
),
|
||||
LLMResponse(content="used another path", tool_calls=[], usage=None),
|
||||
])
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
tools.execute = AsyncMock(side_effect=RuntimeError("boom"))
|
||||
tools.execute = AsyncMock(return_value=ToolResult.error("Error: File not found: x"))
|
||||
|
||||
runner = AgentRunner()
|
||||
result = await runner.run(make_run_spec(provider,
|
||||
initial_messages=[{"role": "user", "content": "do task"}],
|
||||
tools=tools,
|
||||
model="test-model",
|
||||
max_iterations=1,
|
||||
max_iterations=2,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
fail_on_tool_error=True,
|
||||
))
|
||||
|
||||
assert result.final_content == "Error: RuntimeError: boom"
|
||||
assert result.stop_reason == "tool_error"
|
||||
assert provider.chat_with_retry.await_count == 2
|
||||
assert result.final_content == "used another path"
|
||||
assert result.stop_reason == "completed"
|
||||
assert result.tool_events == [
|
||||
{"name": "read_file", "status": "error", "detail": "Error: File not found: x"}
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -241,7 +251,6 @@ async def test_runner_preserves_successful_exec_output_that_starts_with_error():
|
||||
model="test-model",
|
||||
max_iterations=2,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
fail_on_tool_error=True,
|
||||
))
|
||||
|
||||
assert result.final_content == "done"
|
||||
@@ -252,9 +261,8 @@ async def test_runner_preserves_successful_exec_output_that_starts_with_error():
|
||||
|
||||
|
||||
@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
|
||||
to messages so the session never contains orphan tool_calls (#2943)."""
|
||||
async def test_runner_preserves_tool_error_results_in_messages():
|
||||
"""Tool errors stay paired with their calls so the model can recover (#2943)."""
|
||||
from nanobot.agent.runner import AgentRunner
|
||||
|
||||
provider = MagicMock(spec=LLMProvider)
|
||||
@@ -292,11 +300,10 @@ async def test_runner_tool_error_preserves_tool_results_in_messages():
|
||||
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"
|
||||
# Both tool results must be in messages even though tc2 had a fatal error.
|
||||
assert result.stop_reason == "max_iterations"
|
||||
# Both tool results must be in messages even though tc2 returned an error.
|
||||
tool_msgs = [m for m in result.messages if m.get("role") == "tool"]
|
||||
assert len(tool_msgs) == 2
|
||||
assert tool_msgs[0]["tool_call_id"] == "tc1"
|
||||
|
||||
@@ -1448,7 +1448,7 @@ async def test_dispatch_republishes_leftover_queue_messages(tmp_path):
|
||||
"""Messages left in the pending queue after _dispatch are re-published to the bus.
|
||||
|
||||
This tests the finally-block cleanup that prevents message loss when
|
||||
the runner exits early (e.g., max_iterations, tool_error) with messages
|
||||
the runner exits early (e.g., max_iterations) with messages
|
||||
still in the queue.
|
||||
"""
|
||||
from nanobot.bus.events import InboundMessage
|
||||
@@ -1488,8 +1488,8 @@ async def test_dispatch_republishes_leftover_queue_messages(tmp_path):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_drain_injections_on_fatal_tool_error():
|
||||
"""A fatal tool error must not leak recovered content into an injected follow-up."""
|
||||
async def test_drain_injections_after_recoverable_tool_error():
|
||||
"""A tool error and injected follow-up continue in the same runner conversation."""
|
||||
from nanobot.agent.runner import AgentRunner
|
||||
from nanobot.bus.events import InboundMessage
|
||||
|
||||
@@ -1532,7 +1532,6 @@ async def test_drain_injections_on_fatal_tool_error():
|
||||
model="test-model",
|
||||
max_iterations=5,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
fail_on_tool_error=True,
|
||||
injection_callback=inject_cb,
|
||||
))
|
||||
|
||||
|
||||
@@ -398,24 +398,27 @@ async def test_runner_rejects_openai_responses_array_arguments_without_executing
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_treats_legacy_entry_point_error_prefix_as_tool_error(tmp_path):
|
||||
async def test_runner_returns_legacy_entry_point_error_to_model(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=None,
|
||||
))
|
||||
provider.chat_with_retry = AsyncMock(side_effect=[
|
||||
LLMResponse(
|
||||
content="working",
|
||||
tool_calls=[ToolCallRequest(id="call_1", name="legacy_plugin", arguments={})],
|
||||
usage=None,
|
||||
),
|
||||
LLMResponse(content="reported plugin failure", tool_calls=[], usage=None),
|
||||
])
|
||||
|
||||
result = await AgentRunner().run(make_run_spec(provider,
|
||||
initial_messages=[{"role": "user", "content": "run plugin"}],
|
||||
tools=_load_entry_point_plugin(_LegacyErrorPluginTool, tmp_path),
|
||||
model="test-model",
|
||||
max_iterations=1,
|
||||
max_iterations=2,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
fail_on_tool_error=True,
|
||||
))
|
||||
|
||||
assert result.stop_reason == "tool_error"
|
||||
assert result.stop_reason == "completed"
|
||||
assert result.final_content == "reported plugin failure"
|
||||
assert result.tool_events == [
|
||||
{"name": "legacy_plugin", "status": "error", "detail": "Error: legacy plugin failed"}
|
||||
]
|
||||
@@ -441,7 +444,6 @@ async def test_runner_preserves_structured_plugin_success_that_starts_with_error
|
||||
model="test-model",
|
||||
max_iterations=2,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
fail_on_tool_error=True,
|
||||
))
|
||||
|
||||
assert result.stop_reason == "completed"
|
||||
|
||||
@@ -12,7 +12,7 @@ from nanobot.agent.tools.filesystem import FileToolsConfig
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.config.schema import ToolsConfig
|
||||
from nanobot.llm_usage.context import llm_usage_source
|
||||
from nanobot.providers.base import GenerationSettings, LLMProvider
|
||||
from nanobot.providers.base import GenerationSettings, LLMProvider, LLMResponse, ToolCallRequest
|
||||
from nanobot.security.workspace_access import build_workspace_scope
|
||||
from nanobot.utils.llm_runtime import LLMRuntime
|
||||
|
||||
@@ -168,38 +168,36 @@ async def test_subagent_keeps_project_runtime_scope_with_agent_owned_tools(tmp_p
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_subagent_forwards_fail_on_tool_error_to_runner(tmp_path):
|
||||
async def test_subagent_recovers_from_tool_error_in_same_run(tmp_path):
|
||||
provider = MagicMock(spec=LLMProvider)
|
||||
provider.get_default_model.return_value = "test"
|
||||
provider.chat_with_retry = AsyncMock(side_effect=[
|
||||
LLMResponse(
|
||||
content="reading",
|
||||
tool_calls=[
|
||||
ToolCallRequest(
|
||||
id="call_1",
|
||||
name="read_file",
|
||||
arguments={"path": "missing.txt"},
|
||||
)
|
||||
],
|
||||
),
|
||||
LLMResponse(content="recovered without restarting", tool_calls=[]),
|
||||
])
|
||||
sm = SubagentManager(
|
||||
workspace=tmp_path,
|
||||
bus=MessageBus(),
|
||||
max_tool_result_chars=16_000,
|
||||
fail_on_tool_error=False,
|
||||
)
|
||||
sm.runner.run = AsyncMock(
|
||||
return_value=AgentRunResult(final_content="ok", messages=[], stop_reason="completed")
|
||||
)
|
||||
sm._announce_result = AsyncMock()
|
||||
|
||||
status = SubagentStatus(
|
||||
task_id="t1",
|
||||
label="label",
|
||||
task_description="task",
|
||||
started_at=0.0,
|
||||
)
|
||||
|
||||
await sm._run_subagent(
|
||||
"t1",
|
||||
"task",
|
||||
"label",
|
||||
{"channel": "cli", "chat_id": "direct"},
|
||||
status,
|
||||
_runtime(provider),
|
||||
result = await sm.run_inline(
|
||||
task="recover after a missing file",
|
||||
session_key="test:direct",
|
||||
runtime=_runtime(provider),
|
||||
)
|
||||
|
||||
spec = sm.runner.run.call_args.args[0]
|
||||
assert spec.fail_on_tool_error is False
|
||||
assert result == "recovered without restarting"
|
||||
assert provider.chat_with_retry.await_count == 2
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -368,21 +368,6 @@ class TestRunSubagent:
|
||||
mock_announce.assert_called_once()
|
||||
assert mock_announce.call_args.args[-2] == "ok"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_tool_error_run(self, tmp_path):
|
||||
sm = _manager(tmp_path)
|
||||
sm.runner.run = AsyncMock(return_value=AgentRunResult(
|
||||
final_content=None, messages=[], stop_reason="tool_error",
|
||||
tool_events=[{"name": "read_file", "status": "error", "detail": "not found"}],
|
||||
))
|
||||
status = SubagentStatus(task_id="t1", label="label", task_description="do task", started_at=time.monotonic())
|
||||
with patch.object(sm, "_announce_result", new_callable=AsyncMock) as mock_announce:
|
||||
await sm._run_subagent(
|
||||
"t1", "do task", "label",
|
||||
{"channel": "cli", "chat_id": "direct"}, status, _runtime(),
|
||||
)
|
||||
assert mock_announce.call_args.args[-2] == "error"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_exception_run(self, tmp_path):
|
||||
sm = _manager(tmp_path)
|
||||
@@ -504,73 +489,6 @@ class TestAnnounceResult:
|
||||
assert published[0].metadata["origin_message_id"] == "msg-123"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _format_partial_progress
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestFormatPartialProgress:
|
||||
def _make_result(self, tool_events=None, error=None):
|
||||
return MagicMock(tool_events=tool_events or [], error=error)
|
||||
|
||||
def test_completed_only(self):
|
||||
result = self._make_result(tool_events=[
|
||||
{"name": "read_file", "status": "ok", "detail": "file content"},
|
||||
{"name": "exec", "status": "ok", "detail": "output"},
|
||||
])
|
||||
text = SubagentManager._format_partial_progress(result)
|
||||
assert "Completed steps:" in text
|
||||
assert "read_file" in text
|
||||
assert "exec" in text
|
||||
|
||||
def test_failure_only(self):
|
||||
result = self._make_result(tool_events=[
|
||||
{"name": "read_file", "status": "error", "detail": "not found"},
|
||||
])
|
||||
text = SubagentManager._format_partial_progress(result)
|
||||
assert "Failure:" in text
|
||||
assert "not found" in text
|
||||
|
||||
def test_completed_and_failure(self):
|
||||
result = self._make_result(tool_events=[
|
||||
{"name": "read_file", "status": "ok", "detail": "content"},
|
||||
{"name": "exec", "status": "error", "detail": "timeout"},
|
||||
])
|
||||
text = SubagentManager._format_partial_progress(result)
|
||||
assert "Completed steps:" in text
|
||||
assert "Failure:" in text
|
||||
|
||||
def test_limited_to_last_three(self):
|
||||
result = self._make_result(tool_events=[
|
||||
{"name": f"tool_{i}", "status": "ok", "detail": f"result_{i}"}
|
||||
for i in range(5)
|
||||
])
|
||||
text = SubagentManager._format_partial_progress(result)
|
||||
assert "tool_2" in text
|
||||
assert "tool_3" in text
|
||||
assert "tool_4" in text
|
||||
assert "tool_0" not in text
|
||||
assert "tool_1" not in text
|
||||
|
||||
def test_error_without_failure_event(self):
|
||||
result = self._make_result(
|
||||
tool_events=[{"name": "read_file", "status": "ok", "detail": "ok"}],
|
||||
error="Something went wrong",
|
||||
)
|
||||
text = SubagentManager._format_partial_progress(result)
|
||||
assert "Something went wrong" in text
|
||||
|
||||
def test_empty_events_with_error(self):
|
||||
result = self._make_result(error="Total failure")
|
||||
text = SubagentManager._format_partial_progress(result)
|
||||
assert "Total failure" in text
|
||||
|
||||
def test_empty_no_error_returns_fallback(self):
|
||||
result = self._make_result()
|
||||
text = SubagentManager._format_partial_progress(result)
|
||||
assert "Error" in text
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# cancel_by_session
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -450,7 +450,9 @@ class TestSubagentCancellation:
|
||||
mgr._announce_result.assert_awaited_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_subagent_announces_error_when_tool_execution_fails(self, monkeypatch, tmp_path):
|
||||
async def test_subagent_announces_success_after_recovering_from_tool_failure(
|
||||
self, monkeypatch, tmp_path
|
||||
):
|
||||
from nanobot.agent.subagent import SubagentManager
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.providers.base import LLMResponse, ToolCallRequest
|
||||
@@ -458,10 +460,21 @@ class TestSubagentCancellation:
|
||||
bus = MessageBus()
|
||||
provider = MagicMock()
|
||||
provider.get_default_model.return_value = "test-model"
|
||||
provider.chat_with_retry = AsyncMock(return_value=LLMResponse(
|
||||
content="thinking",
|
||||
tool_calls=[ToolCallRequest(id="call_1", name="list_dir", arguments={"path": "."})],
|
||||
))
|
||||
provider.chat_with_retry = AsyncMock(side_effect=[
|
||||
LLMResponse(
|
||||
content="first attempt",
|
||||
tool_calls=[
|
||||
ToolCallRequest(id="call_1", name="list_dir", arguments={"path": "."})
|
||||
],
|
||||
),
|
||||
LLMResponse(
|
||||
content="retrying",
|
||||
tool_calls=[
|
||||
ToolCallRequest(id="call_2", name="list_dir", arguments={"path": "."})
|
||||
],
|
||||
),
|
||||
LLMResponse(content="recovered after tool failure", tool_calls=[]),
|
||||
])
|
||||
mgr = SubagentManager(
|
||||
workspace=tmp_path,
|
||||
bus=bus,
|
||||
@@ -492,11 +505,10 @@ class TestSubagentCancellation:
|
||||
|
||||
mgr._announce_result.assert_awaited_once()
|
||||
args = mgr._announce_result.await_args.args
|
||||
assert "Completed steps:" in args[3]
|
||||
assert "- list_dir: first result" in args[3]
|
||||
assert "Failure:" in args[3]
|
||||
assert "- list_dir: boom" in args[3]
|
||||
assert args[5] == "error"
|
||||
assert args[3] == "recovered after tool failure"
|
||||
assert args[5] == "ok"
|
||||
assert calls["n"] == 2
|
||||
assert provider.chat_with_retry.await_count == 3
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cancel_by_session_cancels_running_subagent_tool(self, monkeypatch, tmp_path):
|
||||
|
||||
@@ -124,6 +124,35 @@ def test_save_config_drops_legacy_max_messages(tmp_path) -> None:
|
||||
assert "max_messages" not in saved["agents"]["defaults"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("field_name", ["failOnToolError", "fail_on_tool_error"])
|
||||
def test_load_config_ignores_removed_fail_on_tool_error(tmp_path, field_name) -> None:
|
||||
config_path = tmp_path / "config.json"
|
||||
config_path.write_text(
|
||||
json.dumps({"agents": {"defaults": {field_name: True, "maxTokens": 1234}}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
config = load_config(config_path)
|
||||
|
||||
assert config.agents.defaults.max_tokens == 1234
|
||||
assert not hasattr(config.agents.defaults, "fail_on_tool_error")
|
||||
|
||||
|
||||
def test_save_config_drops_removed_fail_on_tool_error(tmp_path) -> None:
|
||||
config_path = tmp_path / "config.json"
|
||||
config_path.write_text(
|
||||
json.dumps({"agents": {"defaults": {"failOnToolError": True}}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
config = load_config(config_path)
|
||||
save_config(config, config_path)
|
||||
saved = json.loads(config_path.read_text(encoding="utf-8"))
|
||||
|
||||
assert "failOnToolError" not in saved["agents"]["defaults"]
|
||||
assert "fail_on_tool_error" not in saved["agents"]["defaults"]
|
||||
|
||||
|
||||
def test_onboard_refresh_backfills_missing_channel_fields(tmp_path, monkeypatch) -> None:
|
||||
from nanobot.channels.plugin import load_channel_package
|
||||
|
||||
|
||||
Reference in New Issue
Block a user