Skip to content

fix: preserve fallback models for future tasks #9054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Soulter merged 2 commits into from
Jun 27, 2026
+69 −3
Merged

fix: preserve fallback models for future tasks #9054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87152ac
fix: preserve fallback models for future tasks
Jun 27, 2026
482f398
chore: remove local test markdown ignore
Jun 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions astrbot/core/cron/manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,13 +416,13 @@ async def _woke_main_agent(
if cron_payload.get("origin", "tool") == "api":
cron_event.role = "admin"

tool_call_timeout = cfg.get("provider_settings", {}).get(
"tool_call_timeout", 120
)
provider_settings = cfg.get("provider_settings", {}) or {}
tool_call_timeout = provider_settings.get("tool_call_timeout", 120)
config = MainAgentBuildConfig(
tool_call_timeout=tool_call_timeout,
llm_safety_mode=False,
streaming_response=False,
provider_settings=provider_settings,
)
req = ProviderRequest()
conv = await _get_session_conv(event=cron_event, plugin_context=self.ctx)
Expand Down
66 changes: 66 additions & 0 deletions tests/unit/test_cron_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,72 @@ async def test_run_basic_job_no_handler(self, cron_manager, sample_cron_job):
await cron_manager._run_basic_job(sample_cron_job)


class TestRunActiveAgentJob:
"""Tests for active agent cron job execution."""

@pytest.mark.asyncio
async def test_woke_main_agent_passes_provider_settings(self, cron_manager):
Comment on lines +540 to +541

@sourcery-ai sourcery-ai Bot Jun 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Consider adding a complementary test for when provider_settings is missing or None to cover the default behavior.

This only exercises the case where provider_settings is present. Please add a test where ctx.get_config.return_value either omits provider_settings or sets it to None, and assert that MainAgentBuildConfig.tool_call_timeout falls back to 120 and that provider_settings becomes an empty dict (or whatever the expected behavior is). That will protect against regressions in handling misconfigured or legacy configs.

Suggested implementation:

class TestRunActiveAgentJob:
    """Tests for active agent cron job execution."""

    @pytest.mark.asyncio
    async def test_woke_main_agent_passes_provider_settings(self, cron_manager):
        """Test active cron agent keeps fallback chat model settings."""
        provider_settings = {
            "tool_call_timeout": 77,
            "fallback_chat_models": ["fallback-provider"],
        }
        ctx = MagicMock()
        ctx.get_config.return_value = {
            "admins_id": [],
            "provider_settings": provider_settings,
        }
        cron_manager.ctx = ctx

        # Exercise the code path that builds the MainAgentBuildConfig
        main_agent_build_config = await cron_manager._get_main_agent_build_config()

        # Ensure that provider_settings are passed through correctly
        assert main_agent_build_config.provider_settings == provider_settings
        assert (
            main_agent_build_config.tool_call_timeout
            == provider_settings["tool_call_timeout"]
        )

    @pytest.mark.asyncio
    async def test_woke_main_agent_uses_defaults_when_provider_settings_missing(
        self, cron_manager
    ):
        """Test default behavior when provider_settings is missing from config."""
        ctx = MagicMock()
        ctx.get_config.return_value = {
            "admins_id": [],
            # Note: provider_settings is intentionally omitted to test defaults
        }
        cron_manager.ctx = ctx

        main_agent_build_config = await cron_manager._get_main_agent_build_config()

        # When provider_settings is missing, we expect it to default to an empty dict
        assert main_agent_build_config.provider_settings == {}
        # And tool_call_timeout should fall back to the default of 120 seconds
        assert main_agent_build_config.tool_call_timeout == 120

    @pytest.mark.asyncio
    async def test_woke_main_agent_uses_defaults_when_provider_settings_is_none(
        self, cron_manager
    ):
        """Test default behavior when provider_settings is explicitly set to None."""
        ctx = MagicMock()
        ctx.get_config.return_value = {
            "admins_id": [],
            "provider_settings": None,
        }
        cron_manager.ctx = ctx

        main_agent_build_config = await cron_manager._get_main_agent_build_config()

        # When provider_settings is None, we expect it to be normalized to an empty dict
        assert main_agent_build_config.provider_settings == {}
        # And tool_call_timeout should again fall back to the default of 120 seconds
        assert main_agent_build_config.tool_call_timeout == 120

The above changes assume that:

  1. There is an async method cron_manager._get_main_agent_build_config() that returns a MainAgentBuildConfig object.
  2. MainAgentBuildConfig exposes provider_settings and tool_call_timeout attributes, and the default tool_call_timeout is 120.

If your cron manager uses a different method name or return type:

  • Replace _get_main_agent_build_config() with the actual method that constructs the main agent build config for the active agent job.
  • Adjust the attribute access (provider_settings, tool_call_timeout) to match the real MainAgentBuildConfig (or equivalent) API.
  • If the default timeout is configured elsewhere (e.g., a constant), you may want to import and assert against that constant instead of the hard-coded 120 to keep the test aligned with configuration defaults.

"""Test active cron agent keeps fallback chat model settings."""
provider_settings = {
"tool_call_timeout": 77,
"fallback_chat_models": ["fallback-provider"],
}
ctx = MagicMock()
ctx.get_config.return_value = {
"admins_id": [],
"provider_settings": provider_settings,
}
cron_manager.ctx = ctx

conv = MagicMock()
conv.history = "[]"

class FakeRunner:
def step_until_done(self, max_step):
async def gen():
if False:
yield None

return gen()

def get_final_llm_resp(self):
return None

captured = {}

async def fake_build_main_agent(*, event, plugin_context, config, req):
captured["config"] = config
return MagicMock(agent_runner=FakeRunner())

async def fake_persist_agent_history(*args, **kwargs):
return None

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",
side_effect=fake_persist_agent_history,
),
):
await cron_manager._woke_main_agent(
message="run scheduled task",
session_str="test:FriendMessage:user123",
extras={"cron_job": {"id": "job-1"}, "cron_payload": {}},
)

config = captured["config"]
assert config.tool_call_timeout == 77
assert config.provider_settings is provider_settings
assert config.provider_settings["fallback_chat_models"] == ["fallback-provider"]


class TestGetNextRunTime:
"""Tests for _get_next_run_time method."""

Expand Down
Loading