From 3c20d16117b07255f22e60350f2fef29a3f358e3 Mon Sep 17 00:00:00 2001 From: hanyuanling Date: Wed, 29 Apr 2026 17:53:28 +0800 Subject: [PATCH] fix subagent max iteration limit --- nanobot/agent/loop.py | 7 + nanobot/agent/subagent.py | 10 +- nanobot/agent/tools/self.py | 2 + .../tools/test_self_tool_runtime_sync.py | 29 +++++ tests/agent/tools/test_subagent_tools.py | 122 +++++++++++++++++- 5 files changed, 165 insertions(+), 5 deletions(-) create mode 100644 tests/agent/tools/test_self_tool_runtime_sync.py diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index c03d80651..cbddfc286 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -258,6 +258,7 @@ class AgentLoop: exec_config=self.exec_config, restrict_to_workspace=restrict_to_workspace, disabled_skills=disabled_skills, + max_iterations=self.max_iterations, ) self._unified_session = unified_session self._max_messages = max_messages if max_messages > 0 else 120 @@ -307,6 +308,10 @@ class AgentLoop: self.commands = CommandRouter() 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: """Swap model/provider for future turns without disturbing an active one.""" provider = snapshot.provider @@ -531,6 +536,8 @@ class AgentLoop: Returns (final_content, tools_used, messages, stop_reason, had_injections). """ + self._sync_subagent_runtime_limits() + loop_hook = _LoopHook( self, on_progress=on_progress, diff --git a/nanobot/agent/subagent.py b/nanobot/agent/subagent.py index c100d205b..e64dc8f97 100644 --- a/nanobot/agent/subagent.py +++ b/nanobot/agent/subagent.py @@ -20,7 +20,7 @@ from nanobot.agent.tools.shell import ExecTool from nanobot.agent.tools.web import WebFetchTool, WebSearchTool from nanobot.bus.events import InboundMessage 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.utils.prompt_templates import render_template @@ -81,6 +81,7 @@ class SubagentManager: exec_config: "ExecToolConfig | None" = None, restrict_to_workspace: bool = False, disabled_skills: list[str] | None = None, + max_iterations: int | None = None, ): self.provider = provider self.workspace = workspace @@ -91,6 +92,11 @@ class SubagentManager: self.exec_config = exec_config or ExecToolConfig() self.restrict_to_workspace = restrict_to_workspace 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._running_tasks: dict[str, asyncio.Task[None]] = {} self._task_statuses: dict[str, SubagentStatus] = {} @@ -202,7 +208,7 @@ class SubagentManager: initial_messages=messages, tools=tools, model=self.model, - max_iterations=15, + max_iterations=self.max_iterations, max_tool_result_chars=self.max_tool_result_chars, hook=_SubagentHook(task_id, status), max_iterations_message="Task completed but no final response was generated.", diff --git a/nanobot/agent/tools/self.py b/nanobot/agent/tools/self.py index f05dbf217..59ece04e7 100644 --- a/nanobot/agent/tools/self.py +++ b/nanobot/agent/tools/self.py @@ -394,6 +394,8 @@ class MyTool(Tool): if "min_len" in spec and len(str(value)) < spec["min_len"]: return f"Error: '{key}' must be at least {spec['min_len']} characters" 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}") return f"Set {key} = {value!r} (was {old!r})" diff --git a/tests/agent/tools/test_self_tool_runtime_sync.py b/tests/agent/tools/test_self_tool_runtime_sync.py new file mode 100644 index 000000000..8f65023ff --- /dev/null +++ b/tests/agent/tools/test_self_tool_runtime_sync.py @@ -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 diff --git a/tests/agent/tools/test_subagent_tools.py b/tests/agent/tools/test_subagent_tools.py index bfe845395..a050a4271 100644 --- a/tests/agent/tools/test_subagent_tools.py +++ b/tests/agent/tools/test_subagent_tools.py @@ -54,11 +54,129 @@ async def test_subagent_exec_tool_receives_allowed_env_keys(tmp_path): 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 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.""" from nanobot.agent.loop import AgentLoop - from nanobot.agent.subagent import SubagentManager from nanobot.bus.events import InboundMessage from nanobot.bus.queue import MessageBus from nanobot.session.manager import Session @@ -74,8 +192,6 @@ async def test_drain_pending_blocks_while_subagents_running(tmp_path): injection_callback = None # Capture the injection_callback that _run_agent_loop creates - original_run = loop.runner.run - async def fake_runner_run(spec): nonlocal injection_callback injection_callback = spec.injection_callback