mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-05-30 13:31:12 +00:00
fix subagent max iteration limit
This commit is contained in:
parent
f8fd9f0011
commit
3c20d16117
@ -258,6 +258,7 @@ class AgentLoop:
|
|||||||
exec_config=self.exec_config,
|
exec_config=self.exec_config,
|
||||||
restrict_to_workspace=restrict_to_workspace,
|
restrict_to_workspace=restrict_to_workspace,
|
||||||
disabled_skills=disabled_skills,
|
disabled_skills=disabled_skills,
|
||||||
|
max_iterations=self.max_iterations,
|
||||||
)
|
)
|
||||||
self._unified_session = unified_session
|
self._unified_session = unified_session
|
||||||
self._max_messages = max_messages if max_messages > 0 else 120
|
self._max_messages = max_messages if max_messages > 0 else 120
|
||||||
@ -307,6 +308,10 @@ class AgentLoop:
|
|||||||
self.commands = CommandRouter()
|
self.commands = CommandRouter()
|
||||||
register_builtin_commands(self.commands)
|
register_builtin_commands(self.commands)
|
||||||
|
|
||||||
|
def _sync_subagent_runtime_limits(self) -> None:
|
||||||
|
"""Keep subagent runtime limits aligned with mutable loop settings."""
|
||||||
|
self.subagents.max_iterations = self.max_iterations
|
||||||
|
|
||||||
def _apply_provider_snapshot(self, snapshot: ProviderSnapshot) -> None:
|
def _apply_provider_snapshot(self, snapshot: ProviderSnapshot) -> None:
|
||||||
"""Swap model/provider for future turns without disturbing an active one."""
|
"""Swap model/provider for future turns without disturbing an active one."""
|
||||||
provider = snapshot.provider
|
provider = snapshot.provider
|
||||||
@ -531,6 +536,8 @@ class AgentLoop:
|
|||||||
|
|
||||||
Returns (final_content, tools_used, messages, stop_reason, had_injections).
|
Returns (final_content, tools_used, messages, stop_reason, had_injections).
|
||||||
"""
|
"""
|
||||||
|
self._sync_subagent_runtime_limits()
|
||||||
|
|
||||||
loop_hook = _LoopHook(
|
loop_hook = _LoopHook(
|
||||||
self,
|
self,
|
||||||
on_progress=on_progress,
|
on_progress=on_progress,
|
||||||
|
|||||||
@ -20,7 +20,7 @@ from nanobot.agent.tools.shell import ExecTool
|
|||||||
from nanobot.agent.tools.web import WebFetchTool, WebSearchTool
|
from nanobot.agent.tools.web import WebFetchTool, WebSearchTool
|
||||||
from nanobot.bus.events import InboundMessage
|
from nanobot.bus.events import InboundMessage
|
||||||
from nanobot.bus.queue import MessageBus
|
from nanobot.bus.queue import MessageBus
|
||||||
from nanobot.config.schema import ExecToolConfig, WebToolsConfig
|
from nanobot.config.schema import AgentDefaults, ExecToolConfig, WebToolsConfig
|
||||||
from nanobot.providers.base import LLMProvider
|
from nanobot.providers.base import LLMProvider
|
||||||
from nanobot.utils.prompt_templates import render_template
|
from nanobot.utils.prompt_templates import render_template
|
||||||
|
|
||||||
@ -81,6 +81,7 @@ class SubagentManager:
|
|||||||
exec_config: "ExecToolConfig | None" = None,
|
exec_config: "ExecToolConfig | None" = None,
|
||||||
restrict_to_workspace: bool = False,
|
restrict_to_workspace: bool = False,
|
||||||
disabled_skills: list[str] | None = None,
|
disabled_skills: list[str] | None = None,
|
||||||
|
max_iterations: int | None = None,
|
||||||
):
|
):
|
||||||
self.provider = provider
|
self.provider = provider
|
||||||
self.workspace = workspace
|
self.workspace = workspace
|
||||||
@ -91,6 +92,11 @@ class SubagentManager:
|
|||||||
self.exec_config = exec_config or ExecToolConfig()
|
self.exec_config = exec_config or ExecToolConfig()
|
||||||
self.restrict_to_workspace = restrict_to_workspace
|
self.restrict_to_workspace = restrict_to_workspace
|
||||||
self.disabled_skills = set(disabled_skills or [])
|
self.disabled_skills = set(disabled_skills or [])
|
||||||
|
self.max_iterations = (
|
||||||
|
max_iterations
|
||||||
|
if max_iterations is not None
|
||||||
|
else AgentDefaults().max_tool_iterations
|
||||||
|
)
|
||||||
self.runner = AgentRunner(provider)
|
self.runner = AgentRunner(provider)
|
||||||
self._running_tasks: dict[str, asyncio.Task[None]] = {}
|
self._running_tasks: dict[str, asyncio.Task[None]] = {}
|
||||||
self._task_statuses: dict[str, SubagentStatus] = {}
|
self._task_statuses: dict[str, SubagentStatus] = {}
|
||||||
@ -202,7 +208,7 @@ class SubagentManager:
|
|||||||
initial_messages=messages,
|
initial_messages=messages,
|
||||||
tools=tools,
|
tools=tools,
|
||||||
model=self.model,
|
model=self.model,
|
||||||
max_iterations=15,
|
max_iterations=self.max_iterations,
|
||||||
max_tool_result_chars=self.max_tool_result_chars,
|
max_tool_result_chars=self.max_tool_result_chars,
|
||||||
hook=_SubagentHook(task_id, status),
|
hook=_SubagentHook(task_id, status),
|
||||||
max_iterations_message="Task completed but no final response was generated.",
|
max_iterations_message="Task completed but no final response was generated.",
|
||||||
|
|||||||
@ -394,6 +394,8 @@ class MyTool(Tool):
|
|||||||
if "min_len" in spec and len(str(value)) < spec["min_len"]:
|
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 f"Error: '{key}' must be at least {spec['min_len']} characters"
|
||||||
setattr(self._loop, key, value)
|
setattr(self._loop, key, value)
|
||||||
|
if key == "max_iterations" and hasattr(self._loop, "_sync_subagent_runtime_limits"):
|
||||||
|
self._loop._sync_subagent_runtime_limits()
|
||||||
self._audit("modify", f"{key}: {old!r} -> {value!r}")
|
self._audit("modify", f"{key}: {old!r} -> {value!r}")
|
||||||
return f"Set {key} = {value!r} (was {old!r})"
|
return f"Set {key} = {value!r} (was {old!r})"
|
||||||
|
|
||||||
|
|||||||
29
tests/agent/tools/test_self_tool_runtime_sync.py
Normal file
29
tests/agent/tools/test_self_tool_runtime_sync.py
Normal file
@ -0,0 +1,29 @@
|
|||||||
|
"""Focused tests for MyTool runtime sync side effects."""
|
||||||
|
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from nanobot.agent.tools.self import MyTool
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_my_tool_max_iterations_syncs_subagent_limit() -> None:
|
||||||
|
loop = MagicMock()
|
||||||
|
loop.max_iterations = 40
|
||||||
|
loop._runtime_vars = {}
|
||||||
|
loop.subagents = MagicMock()
|
||||||
|
loop.subagents.max_iterations = loop.max_iterations
|
||||||
|
|
||||||
|
def _sync_subagent_runtime_limits() -> None:
|
||||||
|
loop.subagents.max_iterations = loop.max_iterations
|
||||||
|
|
||||||
|
loop._sync_subagent_runtime_limits = _sync_subagent_runtime_limits
|
||||||
|
|
||||||
|
tool = MyTool(loop=loop)
|
||||||
|
|
||||||
|
result = await tool.execute(action="set", key="max_iterations", value=80)
|
||||||
|
|
||||||
|
assert "Set max_iterations = 80" in result
|
||||||
|
assert loop.max_iterations == 80
|
||||||
|
assert loop.subagents.max_iterations == 80
|
||||||
@ -54,11 +54,129 @@ async def test_subagent_exec_tool_receives_allowed_env_keys(tmp_path):
|
|||||||
mgr.runner.run.assert_awaited_once()
|
mgr.runner.run.assert_awaited_once()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_subagent_uses_configured_max_iterations(tmp_path):
|
||||||
|
"""Subagents should honor the configured tool-iteration limit."""
|
||||||
|
from nanobot.agent.subagent import SubagentManager, SubagentStatus
|
||||||
|
from nanobot.bus.queue import MessageBus
|
||||||
|
|
||||||
|
bus = MessageBus()
|
||||||
|
provider = MagicMock()
|
||||||
|
provider.get_default_model.return_value = "test-model"
|
||||||
|
mgr = SubagentManager(
|
||||||
|
provider=provider,
|
||||||
|
workspace=tmp_path,
|
||||||
|
bus=bus,
|
||||||
|
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||||
|
max_iterations=37,
|
||||||
|
)
|
||||||
|
mgr._announce_result = AsyncMock()
|
||||||
|
|
||||||
|
async def fake_run(spec):
|
||||||
|
assert spec.max_iterations == 37
|
||||||
|
return SimpleNamespace(
|
||||||
|
stop_reason="done",
|
||||||
|
final_content="done",
|
||||||
|
error=None,
|
||||||
|
tool_events=[],
|
||||||
|
)
|
||||||
|
|
||||||
|
mgr.runner.run = AsyncMock(side_effect=fake_run)
|
||||||
|
|
||||||
|
status = SubagentStatus(
|
||||||
|
task_id="sub-1", label="label", task_description="do task", started_at=time.monotonic()
|
||||||
|
)
|
||||||
|
await mgr._run_subagent(
|
||||||
|
"sub-1", "do task", "label", {"channel": "test", "chat_id": "c1"}, status
|
||||||
|
)
|
||||||
|
|
||||||
|
mgr.runner.run.assert_awaited_once()
|
||||||
|
|
||||||
|
|
||||||
|
def test_subagent_default_max_iterations_matches_agent_defaults(tmp_path):
|
||||||
|
"""Direct SubagentManager construction should use the agent default limit."""
|
||||||
|
from nanobot.agent.subagent import SubagentManager
|
||||||
|
from nanobot.bus.queue import MessageBus
|
||||||
|
|
||||||
|
bus = MessageBus()
|
||||||
|
provider = MagicMock()
|
||||||
|
provider.get_default_model.return_value = "test-model"
|
||||||
|
|
||||||
|
mgr = SubagentManager(
|
||||||
|
provider=provider,
|
||||||
|
workspace=tmp_path,
|
||||||
|
bus=bus,
|
||||||
|
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert mgr.max_iterations == AgentDefaults().max_tool_iterations
|
||||||
|
|
||||||
|
|
||||||
|
def test_agent_loop_passes_max_iterations_to_subagents(tmp_path):
|
||||||
|
"""AgentLoop's configured limit should be shared with spawned subagents."""
|
||||||
|
from nanobot.agent.loop import AgentLoop
|
||||||
|
from nanobot.bus.queue import MessageBus
|
||||||
|
|
||||||
|
bus = MessageBus()
|
||||||
|
provider = MagicMock()
|
||||||
|
provider.get_default_model.return_value = "test-model"
|
||||||
|
|
||||||
|
loop = AgentLoop(
|
||||||
|
bus=bus,
|
||||||
|
provider=provider,
|
||||||
|
workspace=tmp_path,
|
||||||
|
model="test-model",
|
||||||
|
max_iterations=42,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert loop.subagents.max_iterations == 42
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_agent_loop_syncs_updated_max_iterations_before_run(tmp_path):
|
||||||
|
"""Runtime max_iterations changes should be reflected before tool execution."""
|
||||||
|
from nanobot.agent.loop import AgentLoop
|
||||||
|
from nanobot.bus.queue import MessageBus
|
||||||
|
|
||||||
|
bus = MessageBus()
|
||||||
|
provider = MagicMock()
|
||||||
|
provider.get_default_model.return_value = "test-model"
|
||||||
|
|
||||||
|
loop = AgentLoop(
|
||||||
|
bus=bus,
|
||||||
|
provider=provider,
|
||||||
|
workspace=tmp_path,
|
||||||
|
model="test-model",
|
||||||
|
max_iterations=42,
|
||||||
|
)
|
||||||
|
loop.tools.get_definitions = MagicMock(return_value=[])
|
||||||
|
|
||||||
|
async def fake_run(spec):
|
||||||
|
assert spec.max_iterations == 55
|
||||||
|
assert loop.subagents.max_iterations == 55
|
||||||
|
return SimpleNamespace(
|
||||||
|
stop_reason="done",
|
||||||
|
final_content="done",
|
||||||
|
error=None,
|
||||||
|
tool_events=[],
|
||||||
|
messages=[],
|
||||||
|
usage={},
|
||||||
|
had_injections=False,
|
||||||
|
tools_used=[],
|
||||||
|
)
|
||||||
|
|
||||||
|
loop.runner.run = AsyncMock(side_effect=fake_run)
|
||||||
|
loop.max_iterations = 55
|
||||||
|
|
||||||
|
await loop._run_agent_loop([])
|
||||||
|
|
||||||
|
loop.runner.run.assert_awaited_once()
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_drain_pending_blocks_while_subagents_running(tmp_path):
|
async def test_drain_pending_blocks_while_subagents_running(tmp_path):
|
||||||
"""_drain_pending should block when no messages are available but sub-agents are still running."""
|
"""_drain_pending should block when no messages are available but sub-agents are still running."""
|
||||||
from nanobot.agent.loop import AgentLoop
|
from nanobot.agent.loop import AgentLoop
|
||||||
from nanobot.agent.subagent import SubagentManager
|
|
||||||
from nanobot.bus.events import InboundMessage
|
from nanobot.bus.events import InboundMessage
|
||||||
from nanobot.bus.queue import MessageBus
|
from nanobot.bus.queue import MessageBus
|
||||||
from nanobot.session.manager import Session
|
from nanobot.session.manager import Session
|
||||||
@ -74,8 +192,6 @@ async def test_drain_pending_blocks_while_subagents_running(tmp_path):
|
|||||||
injection_callback = None
|
injection_callback = None
|
||||||
|
|
||||||
# Capture the injection_callback that _run_agent_loop creates
|
# Capture the injection_callback that _run_agent_loop creates
|
||||||
original_run = loop.runner.run
|
|
||||||
|
|
||||||
async def fake_runner_run(spec):
|
async def fake_runner_run(spec):
|
||||||
nonlocal injection_callback
|
nonlocal injection_callback
|
||||||
injection_callback = spec.injection_callback
|
injection_callback = spec.injection_callback
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user