Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
76 changes: 76 additions & 0 deletions astrbot/core/cron/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from astrbot.core.cron.events import CronMessageEvent
from astrbot.core.db import BaseDatabase
from astrbot.core.db.po import CronJob
from astrbot.core.message.message_event_result import MessageChain
from astrbot.core.platform.message_session import MessageSession
from astrbot.core.provider.entites import ProviderRequest
from astrbot.core.utils.history_saver import persist_agent_history
Expand Down Expand Up @@ -386,9 +387,84 @@ async def _woke_main_agent(
req=req,
summary_note=summary_note,
)
await self._send_active_agent_fallback_if_needed(
session=session,
req=req,
llm_resp=llm_resp,
cron_meta=cron_meta,
)
if not llm_resp:
logger.warning("Cron job agent got no response")
return
Comment on lines +390 to 398
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.

medium

The call to _send_active_agent_fallback_if_needed is currently placed before the null check for llm_resp. Since the fallback mechanism requires a valid response to proceed, it should be called after ensuring llm_resp is not None. This also prevents redundant warning logs when the agent fails to provide any response, as both the fallback function and the current method would log a warning.

Suggested change
await self._send_active_agent_fallback_if_needed(
session=session,
req=req,
llm_resp=llm_resp,
cron_meta=cron_meta,
)
if not llm_resp:
logger.warning("Cron job agent got no response")
return
if not llm_resp:
logger.warning("Cron job agent got no response")
return
await self._send_active_agent_fallback_if_needed(
session=session,
req=req,
llm_resp=llm_resp,
cron_meta=cron_meta,
)


async def _send_active_agent_fallback_if_needed(
self,
*,
session: MessageSession,
req: ProviderRequest,
llm_resp,
cron_meta: dict,
) -> bool:
if self._agent_sent_message_to_user(req):
logger.info(
"cron active agent fallback skipped agent_sent=True session=%s job_id=%s",
session,
cron_meta.get("id"),
)
return False

text = str(getattr(llm_resp, "completion_text", "") or "").strip()
if not llm_resp or getattr(llm_resp, "role", "") != "assistant" or not text:
logger.warning(
"cron active agent fallback skipped no assistant text session=%s job_id=%s",
session,
cron_meta.get("id"),
)
return False

logger.info(
"cron active agent fallback send start session=%s job_id=%s",
session,
cron_meta.get("id"),
)
try:
ok = await self.ctx.send_message(session, MessageChain().message(text))
logger.info(
"cron active agent fallback send done ok=%s session=%s job_id=%s",
ok,
session,
cron_meta.get("id"),
)
return bool(ok)
except Exception as e: # noqa: BLE001
logger.warning(
"cron active agent fallback send exception session=%s job_id=%s err=%r",
session,
cron_meta.get("id"),
e,
exc_info=True,
)
raise
Comment on lines +439 to +447
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.

medium

The exception is logged here with exc_info=True and then re-raised. The top-level caller _run_job (line 224) also catches this exception and logs it with exc_info=True. This results in duplicate stack traces in the logs for the same failure. Consider removing exc_info=True from this warning to keep the logs cleaner while still preserving the specific context of the fallback failure.

Suggested change
except Exception as e: # noqa: BLE001
logger.warning(
"cron active agent fallback send exception session=%s job_id=%s err=%r",
session,
cron_meta.get("id"),
e,
exc_info=True,
)
raise
except Exception as e: # noqa: BLE001
logger.warning(
"cron active agent fallback send exception session=%s job_id=%s err=%r",
session,
cron_meta.get("id"),
e,
)
raise


@staticmethod
def _agent_sent_message_to_user(req: ProviderRequest) -> bool:
results = getattr(req, "tool_calls_result", None)
if not results:
return False
if not isinstance(results, list):
results = [results]

for result in results:
call_results = getattr(result, "tool_calls_result", None) or []
for call_result in call_results:
content = getattr(call_result, "content", "")
if isinstance(content, list):
content = " ".join(
str(getattr(part, "text", part)) for part in content
)
if "Message sent to session" in str(content):
return True
return False


__all__ = ["CronJobManager"]
81 changes: 80 additions & 1 deletion tests/unit/test_cron_manager.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
"""Tests for CronJobManager."""

from datetime import datetime, timedelta, timezone
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock, patch

import pytest

from astrbot.core.cron.manager import CronJobManager, CronJobSchedulingError
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


@pytest.fixture
Expand Down Expand Up @@ -215,7 +219,7 @@ class TestUpdateJob:
"""Tests for update_job method."""

@pytest.mark.asyncio
async def test_update_job(self, cron_manager, mock_db, sample_cron_job):
async def test_update_job(self, cron_manager, mock_db):
"""Test updating a cron job."""
updated_job = CronJob(
job_id="test-job-id",
Expand Down Expand Up @@ -482,6 +486,81 @@ async def test_run_basic_job_no_handler(self, cron_manager, sample_cron_job):
await cron_manager._run_basic_job(sample_cron_job)


class TestActiveAgentFallback:
"""Tests for active-agent cron fallback delivery."""

@pytest.mark.asyncio
async def test_fallback_sends_final_text_when_agent_did_not_send_message(
self, cron_manager, mock_context
):
cron_manager.ctx = mock_context
mock_context.send_message = AsyncMock(return_value=True)
session = MessageSession(
"test-platform",
MessageType.FRIEND_MESSAGE,
"session-1",
)
req = ProviderRequest()
llm_resp = SimpleNamespace(
role="assistant",
completion_text="定时任务完成。",
)

sent = await cron_manager._send_active_agent_fallback_if_needed(
session=session,
req=req,
llm_resp=llm_resp,
cron_meta={"id": "job-1", "name": "job"},
)

assert sent is True
mock_context.send_message.assert_awaited_once()
assert mock_context.send_message.await_args.args[0] == session
assert (
mock_context.send_message.await_args.args[1].chain[0].text
== "定时任务完成。"
)

@pytest.mark.asyncio
async def test_fallback_skips_when_agent_already_sent_message(
self, cron_manager, mock_context
):
cron_manager.ctx = mock_context
mock_context.send_message = AsyncMock(return_value=True)
session = MessageSession(
"test-platform",
MessageType.FRIEND_MESSAGE,
"session-1",
)
req = ProviderRequest()
req.tool_calls_result = [
SimpleNamespace(
tool_calls_result=[
SimpleNamespace(
content=(
"Message sent to session "
"test-platform:FriendMessage:session-1"
)
)
]
)
]
llm_resp = SimpleNamespace(
role="assistant",
completion_text="定时任务完成。",
)

sent = await cron_manager._send_active_agent_fallback_if_needed(
session=session,
req=req,
llm_resp=llm_resp,
cron_meta={"id": "job-1", "name": "job"},
)

assert sent is False
mock_context.send_message.assert_not_awaited()


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

Expand Down