mirror of
https://github.com/AstrBotDevs/AstrBot.git
synced 2026-08-31 01:40:25 +08:00
fix: apply max_agent_step config to cron and background task agents (#9801)
* fix: apply max_agent_step config to cron and background task agents
The cron (scheduled task) and background task wake-up paths hardcoded
step_until_done(30) since the "extract main agent" refactor (0c5308a),
ignoring provider_settings.max_agent_step.
Read the value via coerce_int_config so invalid values fall back to the
default with a warning instead of crashing or silently degrading.
Fixes #9800
* test: cover max_agent_step propagation in cron and background wake-ups
Add parametrized tests asserting step_until_done receives the configured
max_agent_step on both the cron and background task agent paths, with
fallback coverage for missing, boolean-polluted, and numeric-string
config values.
* fix: clamp max_agent_step to a minimum of 1 in cron and background wake-ups
Non-positive provider_settings.max_agent_step values previously reached
step_until_done as-is: the local runner skipped all steps and forced a
final response, while the DeerFlow runner raised ValueError. Pass
min_value=1 to coerce_int_config so zero/negative values are clamped to
a usable minimum, and cover the zero case in both unit test suites.
This commit is contained in:
@@ -48,6 +48,7 @@ from astrbot.core.tools.computer_tools import (
|
||||
)
|
||||
from astrbot.core.tools.message_tools import SendMessageToUserTool
|
||||
from astrbot.core.utils.astrbot_path import get_astrbot_temp_path
|
||||
from astrbot.core.utils.config_number import coerce_int_config
|
||||
from astrbot.core.utils.history_saver import persist_agent_history
|
||||
from astrbot.core.utils.image_ref_utils import is_supported_image_ref
|
||||
from astrbot.core.utils.string_utils import normalize_and_dedupe_strings
|
||||
@@ -549,6 +550,12 @@ class FunctionToolExecutor(BaseFunctionToolExecutor[AstrAgentContext]):
|
||||
cron_event.role = event.role
|
||||
cfg = ctx.get_config(umo=event.unified_msg_origin) or {}
|
||||
provider_settings = cfg.get("provider_settings") or {}
|
||||
agent_max_step = coerce_int_config(
|
||||
provider_settings.get("max_agent_step", 30),
|
||||
default=30,
|
||||
min_value=1,
|
||||
field_name="provider_settings.max_agent_step",
|
||||
)
|
||||
config = MainAgentBuildConfig(
|
||||
tool_call_timeout=run_context.tool_call_timeout,
|
||||
streaming_response=provider_settings.get("stream", False),
|
||||
@@ -594,7 +601,7 @@ class FunctionToolExecutor(BaseFunctionToolExecutor[AstrAgentContext]):
|
||||
return
|
||||
|
||||
runner = result.agent_runner
|
||||
async for _ in runner.step_until_done(30):
|
||||
async for _ in runner.step_until_done(agent_max_step):
|
||||
# agent will send message to user via using tools
|
||||
pass
|
||||
llm_resp = runner.get_final_llm_resp()
|
||||
|
||||
@@ -18,6 +18,7 @@ from astrbot.core.db.po import CronJob
|
||||
from astrbot.core.platform.message_session import MessageSession
|
||||
from astrbot.core.platform.message_type import MessageType
|
||||
from astrbot.core.provider.entites import ProviderRequest
|
||||
from astrbot.core.utils.config_number import coerce_int_config
|
||||
from astrbot.core.utils.history_saver import persist_agent_history
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -442,6 +443,12 @@ class CronJobManager:
|
||||
|
||||
provider_settings = cfg.get("provider_settings", {}) or {}
|
||||
tool_call_timeout = provider_settings.get("tool_call_timeout", 120)
|
||||
agent_max_step = coerce_int_config(
|
||||
provider_settings.get("max_agent_step", 30),
|
||||
default=30,
|
||||
min_value=1,
|
||||
field_name="provider_settings.max_agent_step",
|
||||
)
|
||||
config = MainAgentBuildConfig(
|
||||
tool_call_timeout=tool_call_timeout,
|
||||
llm_safety_mode=False,
|
||||
@@ -488,7 +495,7 @@ class CronJobManager:
|
||||
return
|
||||
|
||||
runner = result.agent_runner
|
||||
async for _ in runner.step_until_done(30):
|
||||
async for _ in runner.step_until_done(agent_max_step):
|
||||
# agent will send message to user via using tools
|
||||
pass
|
||||
llm_resp = runner.get_final_llm_resp()
|
||||
|
||||
@@ -430,6 +430,85 @@ async def test_background_wakeup_passes_provider_settings_to_main_agent(
|
||||
assert config.provider_settings["fallback_chat_models"] == ["fallback-provider"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("provider_settings", "expected_max_step"),
|
||||
[
|
||||
pytest.param({"max_agent_step": 50}, 50, id="configured"),
|
||||
pytest.param({}, 30, id="missing_falls_back_to_default"),
|
||||
pytest.param({"max_agent_step": True}, 30, id="boolean_falls_back_to_default"),
|
||||
pytest.param({"max_agent_step": "50"}, 50, id="numeric_string_coerced"),
|
||||
pytest.param({"max_agent_step": 0}, 1, id="zero_clamped_to_min"),
|
||||
],
|
||||
)
|
||||
async def test_background_wakeup_applies_max_agent_step(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
provider_settings: dict,
|
||||
expected_max_step: int,
|
||||
):
|
||||
class _StepCapturingRunner:
|
||||
def __init__(self):
|
||||
self.captured_max_step = None
|
||||
|
||||
async def step_until_done(self, max_step):
|
||||
self.captured_max_step = max_step
|
||||
if False:
|
||||
yield
|
||||
|
||||
def get_final_llm_resp(self):
|
||||
return SimpleNamespace(role="assistant", completion_text="done")
|
||||
|
||||
runner = _StepCapturingRunner()
|
||||
|
||||
async def _fake_get_session_conv(**_kwargs):
|
||||
return SimpleNamespace(history="[]")
|
||||
|
||||
async def _fake_build_main_agent(**_kwargs):
|
||||
return SimpleNamespace(agent_runner=runner)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"astrbot.core.astr_main_agent._get_session_conv",
|
||||
_fake_get_session_conv,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"astrbot.core.astr_main_agent.build_main_agent",
|
||||
_fake_build_main_agent,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"astrbot.core.astr_agent_tool_exec.persist_agent_history",
|
||||
AsyncMock(),
|
||||
)
|
||||
|
||||
send_tool = FunctionTool(
|
||||
name="send_message_to_user",
|
||||
description="send",
|
||||
parameters={"type": "object", "properties": {}},
|
||||
)
|
||||
context = SimpleNamespace(
|
||||
get_config=lambda **_kwargs: {"provider_settings": dict(provider_settings)},
|
||||
get_llm_tool_manager=lambda: SimpleNamespace(
|
||||
get_builtin_tool=lambda _tool_cls: send_tool
|
||||
),
|
||||
conversation_manager=SimpleNamespace(),
|
||||
)
|
||||
run_context = ContextWrapper(
|
||||
context=SimpleNamespace(event=_DummyEvent([]), context=context),
|
||||
tool_call_timeout=120,
|
||||
)
|
||||
|
||||
await FunctionToolExecutor._wake_main_agent_for_background_result(
|
||||
run_context,
|
||||
task_id="task-id",
|
||||
tool_name="long_tool",
|
||||
result_text="ok",
|
||||
tool_args={},
|
||||
note="task finished",
|
||||
summary_name="BackgroundTask",
|
||||
)
|
||||
|
||||
assert runner.captured_max_step == expected_max_step
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_collect_handoff_image_urls_filters_extensionless_file_outside_temp_root(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
|
||||
@@ -646,6 +646,76 @@ class TestRunActiveAgentJob:
|
||||
assert config.provider_settings is provider_settings
|
||||
assert config.provider_settings["fallback_chat_models"] == ["fallback-provider"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("provider_settings", "expected_max_step"),
|
||||
[
|
||||
pytest.param({"max_agent_step": 50}, 50, id="configured"),
|
||||
pytest.param({}, 30, id="missing_falls_back_to_default"),
|
||||
pytest.param(
|
||||
{"max_agent_step": True}, 30, id="boolean_falls_back_to_default"
|
||||
),
|
||||
pytest.param({"max_agent_step": "50"}, 50, id="numeric_string_coerced"),
|
||||
pytest.param({"max_agent_step": 0}, 1, id="zero_clamped_to_min"),
|
||||
],
|
||||
)
|
||||
async def test_woke_main_agent_applies_max_agent_step(
|
||||
self, cron_manager, provider_settings, expected_max_step
|
||||
):
|
||||
"""Test the cron agent runner receives max_agent_step from provider settings."""
|
||||
|
||||
class _StepCapturingRunner:
|
||||
def __init__(self):
|
||||
self.captured_max_step = None
|
||||
|
||||
def step_until_done(self, max_step):
|
||||
self.captured_max_step = max_step
|
||||
|
||||
async def gen():
|
||||
if False:
|
||||
yield None
|
||||
|
||||
return gen()
|
||||
|
||||
def get_final_llm_resp(self):
|
||||
return None
|
||||
|
||||
ctx = MagicMock()
|
||||
ctx.get_config.return_value = {
|
||||
"admins_id": [],
|
||||
"provider_settings": dict(provider_settings),
|
||||
}
|
||||
cron_manager.ctx = ctx
|
||||
|
||||
conv = MagicMock()
|
||||
conv.history = "[]"
|
||||
runner = _StepCapturingRunner()
|
||||
|
||||
async def fake_build_main_agent(*, event, plugin_context, config, req):
|
||||
return MagicMock(agent_runner=runner)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"astrbot.core.astr_main_agent._get_session_conv",
|
||||
AsyncMock(return_value=conv),
|
||||
),
|
||||
patch(
|
||||
"astrbot.core.astr_main_agent.build_main_agent",
|
||||
side_effect=fake_build_main_agent,
|
||||
),
|
||||
patch(
|
||||
"astrbot.core.cron.manager.persist_agent_history",
|
||||
AsyncMock(),
|
||||
),
|
||||
):
|
||||
await cron_manager._woke_main_agent(
|
||||
message="run scheduled task",
|
||||
session_str="test:FriendMessage:user123",
|
||||
extras={"cron_job": {"id": "job-1"}, "cron_payload": {}},
|
||||
)
|
||||
|
||||
assert runner.captured_max_step == expected_max_step
|
||||
|
||||
|
||||
class TestGetNextRunTime:
|
||||
"""Tests for _get_next_run_time method."""
|
||||
|
||||
Reference in New Issue
Block a user