fix: make subagent fail_on_tool_error configurable (#4198)
Add fail_on_tool_error to AgentDefaults and wire it through AgentLoop -> SubagentManager -> AgentRunSpec. Previously hardcoded to True in SubagentManager._run_subagent. Now configurable via config.json with default True for backward compatibility. When set to False, subagents can retry on minor tool errors instead of immediately failing. Changes: - nanobot/config/schema.py: add fail_on_tool_error field (default True) - nanobot/agent/subagent.py: accept and forward fail_on_tool_error - nanobot/agent/loop.py: pass config through to SubagentManager - tests/agent/test_subagent.py: add regression test Signed-off-by: axelray-dev <110029405+axelray-dev@users.noreply.github.com>
This commit is contained in:
@@ -190,6 +190,7 @@ class AgentLoop:
|
|||||||
context_window_tokens: int | None = None,
|
context_window_tokens: int | None = None,
|
||||||
context_block_limit: int | None = None,
|
context_block_limit: int | None = None,
|
||||||
max_tool_result_chars: int | None = None,
|
max_tool_result_chars: int | None = None,
|
||||||
|
fail_on_tool_error: bool | None = None,
|
||||||
provider_retry_mode: str = "standard",
|
provider_retry_mode: str = "standard",
|
||||||
tool_hint_max_length: int | None = None,
|
tool_hint_max_length: int | None = None,
|
||||||
cron_service: CronService | None = None,
|
cron_service: CronService | None = None,
|
||||||
@@ -287,6 +288,7 @@ class AgentLoop:
|
|||||||
disabled_skills=disabled_skills,
|
disabled_skills=disabled_skills,
|
||||||
max_iterations=self.max_iterations,
|
max_iterations=self.max_iterations,
|
||||||
max_concurrent_subagents=max_concurrent_subagents,
|
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),
|
llm_wall_timeout_for_session=lambda sk: runner_wall_llm_timeout_s(self.sessions, sk),
|
||||||
)
|
)
|
||||||
self._unified_session = unified_session
|
self._unified_session = unified_session
|
||||||
@@ -377,6 +379,7 @@ class AgentLoop:
|
|||||||
context_window_tokens=context_window_tokens,
|
context_window_tokens=context_window_tokens,
|
||||||
context_block_limit=defaults.context_block_limit,
|
context_block_limit=defaults.context_block_limit,
|
||||||
max_tool_result_chars=defaults.max_tool_result_chars,
|
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,
|
provider_retry_mode=defaults.provider_retry_mode,
|
||||||
tool_hint_max_length=defaults.tool_hint_max_length,
|
tool_hint_max_length=defaults.tool_hint_max_length,
|
||||||
restrict_to_workspace=config.tools.restrict_to_workspace,
|
restrict_to_workspace=config.tools.restrict_to_workspace,
|
||||||
|
|||||||
@@ -86,6 +86,7 @@ class SubagentManager:
|
|||||||
disabled_skills: list[str] | None = None,
|
disabled_skills: list[str] | None = None,
|
||||||
max_iterations: int | None = None,
|
max_iterations: int | None = None,
|
||||||
max_concurrent_subagents: 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,
|
llm_wall_timeout_for_session: Callable[[str | None], float | None] | None = None,
|
||||||
):
|
):
|
||||||
defaults = AgentDefaults()
|
defaults = AgentDefaults()
|
||||||
@@ -107,6 +108,11 @@ class SubagentManager:
|
|||||||
if max_concurrent_subagents is not None
|
if max_concurrent_subagents is not None
|
||||||
else defaults.max_concurrent_subagents
|
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(provider)
|
self.runner = AgentRunner(provider)
|
||||||
self._llm_wall_timeout_for_session = llm_wall_timeout_for_session
|
self._llm_wall_timeout_for_session = llm_wall_timeout_for_session
|
||||||
self._running_tasks: dict[str, asyncio.Task[None]] = {}
|
self._running_tasks: dict[str, asyncio.Task[None]] = {}
|
||||||
@@ -251,7 +257,7 @@ class SubagentManager:
|
|||||||
max_iterations_message="Task completed but no final response was generated.",
|
max_iterations_message="Task completed but no final response was generated.",
|
||||||
finalize_on_max_iterations=False,
|
finalize_on_max_iterations=False,
|
||||||
error_message=None,
|
error_message=None,
|
||||||
fail_on_tool_error=True,
|
fail_on_tool_error=self.fail_on_tool_error,
|
||||||
checkpoint_callback=_on_checkpoint,
|
checkpoint_callback=_on_checkpoint,
|
||||||
session_key=sess_key,
|
session_key=sess_key,
|
||||||
workspace=root,
|
workspace=root,
|
||||||
|
|||||||
@@ -132,6 +132,7 @@ class AgentDefaults(Base):
|
|||||||
fallback_models: list[FallbackCandidate] = Field(default_factory=list)
|
fallback_models: list[FallbackCandidate] = Field(default_factory=list)
|
||||||
max_tool_iterations: int = 200
|
max_tool_iterations: int = 200
|
||||||
max_concurrent_subagents: int = Field(default=1, ge=1)
|
max_concurrent_subagents: int = Field(default=1, ge=1)
|
||||||
|
fail_on_tool_error: bool = True
|
||||||
max_tool_result_chars: int = 16_000
|
max_tool_result_chars: int = 16_000
|
||||||
provider_retry_mode: Literal["standard", "persistent"] = "standard"
|
provider_retry_mode: Literal["standard", "persistent"] = "standard"
|
||||||
tool_hint_max_length: int = Field(
|
tool_hint_max_length: int = Field(
|
||||||
|
|||||||
@@ -1,11 +1,12 @@
|
|||||||
"""Tests for SubagentManager."""
|
"""Tests for SubagentManager."""
|
||||||
|
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import MagicMock
|
from unittest.mock import AsyncMock, MagicMock
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from nanobot.agent.subagent import SubagentManager
|
from nanobot.agent.runner import AgentRunResult
|
||||||
|
from nanobot.agent.subagent import SubagentManager, SubagentStatus
|
||||||
from nanobot.agent.tools.filesystem import FileToolsConfig
|
from nanobot.agent.tools.filesystem import FileToolsConfig
|
||||||
from nanobot.bus.queue import MessageBus
|
from nanobot.bus.queue import MessageBus
|
||||||
from nanobot.config.schema import ToolsConfig
|
from nanobot.config.schema import ToolsConfig
|
||||||
@@ -79,3 +80,33 @@ def test_subagent_respects_file_tool_toggle(tmp_path):
|
|||||||
"write_file",
|
"write_file",
|
||||||
}
|
}
|
||||||
assert file_tools.isdisjoint(tools.tool_names)
|
assert file_tools.isdisjoint(tools.tool_names)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_subagent_forwards_fail_on_tool_error_to_runner(tmp_path):
|
||||||
|
provider = MagicMock(spec=LLMProvider)
|
||||||
|
provider.get_default_model.return_value = "test"
|
||||||
|
sm = SubagentManager(
|
||||||
|
provider=provider,
|
||||||
|
workspace=tmp_path,
|
||||||
|
bus=MessageBus(),
|
||||||
|
model="test",
|
||||||
|
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)
|
||||||
|
|
||||||
|
spec = sm.runner.run.call_args.args[0]
|
||||||
|
assert spec.fail_on_tool_error is False
|
||||||
|
|||||||
Reference in New Issue
Block a user