Skip to content

fix: localize future task times #9462

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
Jul 31, 2026
+234 −5
Merged

fix: localize future task times #9462

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
d34b237
fix: localize future task times
wcqqq1214 Jul 30, 2026
3e6f62b
fix: narrow timezone lookup exception handling
wcqqq1214 Jul 30, 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
18 changes: 18 additions & 0 deletions astrbot/core/cron/manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,24 @@ def _get_next_run_time(self, job_id: str):
return None
return aps_job.next_run_time.astimezone(timezone.utc)

def get_next_run_time(self, job_id: str) -> datetime | None:
"""Read the live next-run time straight from the scheduler.

The DB copy of ``next_run_time`` is written via a fire-and-forget
task in ``_schedule_job``, so it can still be stale/None right after
``add_active_job``/``update_job`` return. The scheduler itself is
updated synchronously, so callers that need an immediate answer
should use this instead of the job row's ``next_run_time`` field.

Args:
job_id: The scheduled job's ID.

Returns:
The job's next scheduled run time in UTC, or None if the job
is not currently scheduled.
"""
return self._get_next_run_time(job_id)

async def run_job_now(self, job_id: str) -> None:
await self._run_job(job_id, ignore_enabled=True, delete_run_once=False)

Expand Down
70 changes: 68 additions & 2 deletions astrbot/core/tools/cron_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
from datetime import datetime
from datetime import timezone as dt_timezone
from typing import Any
from zoneinfo import ZoneInfo, ZoneInfoNotFoundError

from pydantic import Field
from pydantic.dataclasses import dataclass

from astrbot import logger
from astrbot.core.agent.run_context import ContextWrapper
from astrbot.core.agent.tool import FunctionTool, ToolExecResult
from astrbot.core.astr_agent_context import AstrAgentContext
Expand Down Expand Up @@ -130,18 +133,56 @@ async def call(
"origin": "tool",
}

tz_name = str(

@sourcery-ai sourcery-ai Bot Jul 30, 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.

issue (complexity): Consider extracting the repeated timezone resolution and datetime normalization logic into small helper functions to keep cron creation and listing paths simpler and more consistent.

You can reduce the new complexity by centralizing the repeated timezone logic into small helpers, without changing behavior.

1. Extract timezone-from-config helper

The tz_name / tzinfo resolution block is duplicated in create and list. A focused helper keeps it consistent and easier to change later:

from typing import Tuple

def _get_user_timezone(context: ContextWrapper) -> Tuple[str | None, ZoneInfo | None]:
    tz_name = str(
        context.context.context.get_config(
            umo=context.context.event.unified_msg_origin
        ).get("timezone")
        or ""
    ).strip()
    if not tz_name:
        return None, None

    try:
        return tz_name, ZoneInfo(tz_name)
    except Exception:
        logger.warning(
            "Invalid timezone %r in config, falling back to system timezone.",
            tz_name,
        )
        return tz_name, None

Then in both branches:

# create path
tz_name, tzinfo = _get_user_timezone(context)
job = await cron_mgr.add_active_job(
    ...,
    timezone=tz_name or None,
    ...
)

# list path
tz_name, tzinfo = _get_user_timezone(context)

2. Extract UTC→display-timezone normalization helper

The next_run normalization is conceptually the same in both paths. A helper for “stored/scheduler UTC (possibly naive) → display timezone” removes duplication and inline branching:

from datetime import datetime, timezone as dt_timezone

def _to_display_timezone(dt: datetime | None, tzinfo: ZoneInfo | None) -> datetime | None:
    if dt is None:
        return None
    # DB/scheduler values are UTC, possibly naive
    if dt.tzinfo is None:
        dt = dt.replace(tzinfo=dt_timezone.utc)
    return dt.astimezone(tzinfo) if tzinfo else dt.astimezone()

Use it in both places:

# create path
next_run = cron_mgr.get_next_run_time(job.job_id) or job.next_run_time
if next_run is not None:
    next_run = _to_display_timezone(next_run, tzinfo)
elif run_at_dt is not None:
    # keep the existing "run_at is in display timezone if naive" behavior
    next_run = (
        run_at_dt.astimezone(tzinfo)
        if run_at_dt.tzinfo
        else (run_at_dt.replace(tzinfo=tzinfo) if tzinfo else run_at_dt)
    )

# list path
next_run = _to_display_timezone(j.next_run_time, tzinfo)

This preserves all current behavior (including the run_at fallback and naive handling) but removes duplicated logic and separates concerns: config → timezone, stored UTC → display timezone.

context.context.context.get_config(
umo=context.context.event.unified_msg_origin
).get("timezone")
or ""
).strip()
tzinfo = None
if tz_name:
try:
tzinfo = ZoneInfo(tz_name)
except ZoneInfoNotFoundError:
logger.warning(
"Invalid timezone %r in config, falling back to system timezone.",
tz_name,
)
Comment thread
sourcery-ai[bot] marked this conversation as resolved.

try:
job = await cron_mgr.add_active_job(
name=name,
cron_expression=str(cron_expression) if cron_expression else None,
payload=payload,
description=note,
timezone=tz_name or None,
run_once=run_once,
run_at=run_at_dt,
)
except CronJobSchedulingError:
return "error: failed to schedule task due to invalid configuration."
next_run = job.next_run_time or run_at_dt
# add_active_job writes next_run_time to the DB via a fire-and-forget
# task, so job.next_run_time can still be None here; read the live
# value straight from the scheduler instead.
next_run = cron_mgr.get_next_run_time(job.job_id) or job.next_run_time
if next_run is not None:
# Scheduler/DB values are always UTC, even when naive (SQLite
# has no tz-aware column type), so restore the UTC label
# before converting to the display timezone.
if next_run.tzinfo is None:
next_run = next_run.replace(tzinfo=dt_timezone.utc)
next_run = (
next_run.astimezone(tzinfo) if tzinfo else next_run.astimezone()
)
elif run_at_dt is not None:
# No scheduler/DB value yet; fall back to the user-supplied
# run_at, treating a naive value as already being in the
# display timezone.
next_run = (
run_at_dt.astimezone(tzinfo)
if run_at_dt.tzinfo
else (run_at_dt.replace(tzinfo=tzinfo) if tzinfo else run_at_dt)
)
suffix = (
f"one-time at {next_run}"
if run_once
Expand Down Expand Up @@ -245,10 +286,35 @@ async def call(
]
if not jobs:
return "No cron jobs found."
tz_name = str(
context.context.context.get_config(
umo=context.context.event.unified_msg_origin
).get("timezone")
or ""
).strip()
tzinfo = None
if tz_name:
try:
tzinfo = ZoneInfo(tz_name)
except ZoneInfoNotFoundError:
logger.warning(
"Invalid timezone %r in config, falling back to system timezone.",
tz_name,
)
lines = []
for j in jobs:
next_run = j.next_run_time
if next_run is not None:
# DB values are always UTC, even when naive (SQLite has
# no tz-aware column type), so restore the UTC label
# before converting to the display timezone.
if next_run.tzinfo is None:
next_run = next_run.replace(tzinfo=dt_timezone.utc)
next_run = (
next_run.astimezone(tzinfo) if tzinfo else next_run.astimezone()
)
lines.append(
f"{j.job_id} | {j.name} | {j.job_type} | run_once={getattr(j, 'run_once', False)} | enabled={j.enabled} | next={j.next_run_time}"
f"{j.job_id} | {j.name} | {j.job_type} | run_once={getattr(j, 'run_once', False)} | enabled={j.enabled} | next={next_run}"
)
return "\n".join(lines)

Expand Down
151 changes: 148 additions & 3 deletions tests/unit/test_cron_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
"""Tests for cron tool metadata."""

from datetime import datetime
from types import SimpleNamespace
from unittest.mock import AsyncMock
from unittest.mock import AsyncMock, MagicMock

import pytest

from astrbot.core.tools.cron_tools import FutureTaskTool


def _context(cron_mgr, *, umo: str = "test:group:shared", sender_id: str = "user-1"):
def _context(
cron_mgr,
*,
umo: str = "test:group:shared",
sender_id: str = "user-1",
tz_name: str | None = "Asia/Shanghai",
):
return SimpleNamespace(
context=SimpleNamespace(
context=SimpleNamespace(cron_manager=cron_mgr),
context=SimpleNamespace(
cron_manager=cron_mgr,
get_config=lambda umo=None: {"timezone": tz_name},
),
event=SimpleNamespace(
unified_msg_origin=umo,
get_sender_id=lambda: sender_id,
Expand Down Expand Up @@ -222,3 +232,138 @@ async def test_future_task_list_filters_by_umo_and_sender():
assert "own-job" in result
assert "other-sender-job" not in result
assert "other-umo-job" not in result


@pytest.mark.asyncio
async def test_future_task_list_localizes_naive_utc_next_run_time_to_shanghai():
"""List mode should treat a naive DB next_run_time as UTC and convert it.

SQLite has no tz-aware datetime column, so the real DB layer always
returns next_run_time without tzinfo even though the stored instant is
UTC. This must not be misread as "already in the display timezone".
"""
tool = FutureTaskTool()
job = _job("job-1")
job.next_run_time = datetime(2026, 1, 1, 0, 0)
cron_mgr = SimpleNamespace(list_jobs=AsyncMock(return_value=[job]))

result = await tool.call(
_context(cron_mgr, tz_name="Asia/Shanghai"),
action="list",
)

assert "2026-01-01 08:00:00+08:00" in result


@pytest.mark.asyncio
async def test_future_task_list_localizes_naive_utc_next_run_time_to_new_york():
"""List mode should honor a differently configured IANA timezone."""
tool = FutureTaskTool()
job = _job("job-1")
job.next_run_time = datetime(2026, 1, 1, 0, 0)
cron_mgr = SimpleNamespace(list_jobs=AsyncMock(return_value=[job]))

result = await tool.call(
_context(cron_mgr, tz_name="America/New_York"),
action="list",
)

assert "2025-12-31 19:00:00-05:00" in result


@pytest.mark.asyncio
async def test_future_task_list_falls_back_when_timezone_invalid():
"""An invalid configured timezone should not crash the tool."""
tool = FutureTaskTool()
job = _job("job-1")
job.next_run_time = datetime(2026, 1, 1, 0, 0)
cron_mgr = SimpleNamespace(list_jobs=AsyncMock(return_value=[job]))

result = await tool.call(
_context(cron_mgr, tz_name="Not/AZone"),
action="list",
)

assert "job-1" in result


@pytest.mark.asyncio
async def test_future_task_create_passes_config_timezone_to_scheduler():
"""Create mode should forward the configured timezone so recurring jobs don't
silently use the server's local timezone."""
tool = FutureTaskTool()
created_job = SimpleNamespace(
job_id="job-1",
name="active_agent_task",
next_run_time=None,
)
cron_mgr = SimpleNamespace(
add_active_job=AsyncMock(return_value=created_job),
get_next_run_time=MagicMock(return_value=datetime(2026, 1, 1, 0, 0)),
)

result = await tool.call(
_context(cron_mgr, tz_name="Asia/Shanghai"),
action="create",
cron_expression="0 8 * * *",
note="daily reminder",
)

cron_mgr.add_active_job.assert_awaited_once()
_, call_kwargs = cron_mgr.add_active_job.call_args
assert call_kwargs["timezone"] == "Asia/Shanghai"
cron_mgr.get_next_run_time.assert_called_once_with("job-1")
assert "2026-01-01 08:00:00+08:00" in result


@pytest.mark.asyncio
async def test_future_task_create_localizes_next_run_for_new_york():
"""Create mode's reported next-run time should reflect a non-Shanghai tz too."""
tool = FutureTaskTool()
created_job = SimpleNamespace(
job_id="job-1",
name="active_agent_task",
next_run_time=None,
)
cron_mgr = SimpleNamespace(
add_active_job=AsyncMock(return_value=created_job),
get_next_run_time=MagicMock(return_value=datetime(2026, 1, 1, 0, 0)),
)

result = await tool.call(
_context(cron_mgr, tz_name="America/New_York"),
action="create",
cron_expression="0 8 * * *",
note="daily reminder",
)

_, call_kwargs = cron_mgr.add_active_job.call_args
assert call_kwargs["timezone"] == "America/New_York"
assert "2025-12-31 19:00:00-05:00" in result


@pytest.mark.asyncio
async def test_future_task_create_falls_back_to_run_at_when_scheduler_has_no_time():
"""If the scheduler has not registered a next-run time yet (e.g. run_once
scheduled far in the future), create mode should fall back to displaying
the user-supplied run_at instead of the literal string 'None'."""
tool = FutureTaskTool()
created_job = SimpleNamespace(
job_id="job-1",
name="active_agent_task",
next_run_time=None,
)
cron_mgr = SimpleNamespace(
add_active_job=AsyncMock(return_value=created_job),
get_next_run_time=MagicMock(return_value=None),
)

result = await tool.call(
_context(cron_mgr, tz_name="Asia/Shanghai"),
action="create",
run_once=True,
run_at="2026-02-02T08:00:00+08:00",
note="one-time reminder",
)

assert "2026-02-02 08:00:00+08:00" in result
Loading