fix: localize future task times

[wcqqq1214]

Fixes #9447

Modifications / 改动点

• Read the live next-run time from APScheduler through a public get_next_run_time method.

• Convert naive SQLite UTC timestamps to the configured display timezone correctly.

• Pass the configured timezone to active-agent scheduling.

• Add regression coverage for naive UTC values, multiple timezones, scheduler lookup, and run_at fallback.

• This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

This is a backend-only change, so screenshots are not applicable.

pytest tests/unit/test_cron_tools.py tests/unit/test_cron_manager.py -q
50 passed

ruff format --check on the changed files — passed.
ruff check on the changed files — passed.

---

Checklist / 检查清单

• 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
  / 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。

• 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
  / 我的更改经过了良好的测试，并已在上方提供了“验证步骤”和“运行截图”。

• 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
  / 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到 requirements.txt 和 pyproject.toml 文件相应位置。

• 😮 My changes do not introduce malicious code.
  / 我的更改没有引入恶意代码。

Summary by Sourcery

Localize cron future task scheduling and listing times using the configured timezone, and ensure next-run times are read reliably from the scheduler instead of potentially stale database values.

Bug Fixes:

• Treat naive SQLite UTC next_run_time values as UTC and convert them to the configured display timezone for both task creation and listing.
• Avoid returning 'None' for newly scheduled or run-once tasks by falling back to scheduler lookup or the user-supplied run_at time.
• Handle invalid timezone configurations gracefully without breaking cron tool behavior.

Enhancements:

• Propagate the configured timezone into active-agent job scheduling to prevent unintended use of the server local timezone.
• Expose a public get_next_run_time method on the cron manager to retrieve live next-run times from APScheduler.

Tests:

• Add regression tests covering naive UTC timestamps, multiple timezones, invalid timezone fallback, scheduler-based next-run lookup, and run_at fallback behavior for future task tools.

[dosubot]

📄 Knowledge review

Dosu skipped reviewing this PR because your organization has used its 200 included credits for the month. Your usage will reset on 2026-08-01. To have Dosu review this PR before then, ask your organization admin to upgrade to a pro account.

---

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• Timezone resolution logic (tz_name/tzinfo via ZoneInfo and logging on failure) is duplicated between the create and list paths; consider extracting this into a small helper to keep behavior consistent and reduce repetition.
• The ZoneInfo lookup is wrapped in a broad except Exception; since ZoneInfo raises a specific ZoneInfoNotFoundError, narrowing the exception type would make the error handling clearer and avoid masking unrelated issues.
• Accessing the timezone via context.context.context.get_config(umo=...) in multiple places is quite nested and fragile; consider adding a helper on ContextWrapper/AstrAgentContext for retrieving the configured timezone to simplify and centralize this logic.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Timezone resolution logic (tz_name/tzinfo via ZoneInfo and logging on failure) is duplicated between the create and list paths; consider extracting this into a small helper to keep behavior consistent and reduce repetition.
- The ZoneInfo lookup is wrapped in a broad `except Exception`; since ZoneInfo raises a specific `ZoneInfoNotFoundError`, narrowing the exception type would make the error handling clearer and avoid masking unrelated issues.
- Accessing the timezone via `context.context.context.get_config(umo=...)` in multiple places is quite nested and fragile; consider adding a helper on ContextWrapper/AstrAgentContext for retrieving the configured timezone to simplify and centralize this logic.

## Individual Comments

### Comment 1
<location path="astrbot/core/tools/cron_tools.py" line_range="144-150" />
<code_context>
+            ).strip()
+            tzinfo = None
+            if tz_name:
+                try:
+                    tzinfo = ZoneInfo(tz_name)
+                except Exception:
+                    logger.warning(
+                        "Invalid timezone %r in config, falling back to system timezone.",
+                        tz_name,
+                    )
+
             try:
</code_context>
<issue_to_address>
**issue (bug_risk):** Catching a broad Exception when constructing ZoneInfo may hide unexpected errors.

This `except Exception:` will catch and hide any error during `ZoneInfo` construction, not just invalid timezone names. Please restrict it to the specific exception(s) raised for unknown/invalid timezones (e.g. `ZoneInfoNotFoundError`) so genuine bugs aren’t silently ignored while still handling bad config values.
</issue_to_address>

### Comment 2
<location path="astrbot/core/tools/cron_tools.py" line_range="136" />
<code_context>
                 "origin": "tool",
             }

+            tz_name = str(
+                context.context.context.get_config(
+                    umo=context.context.event.unified_msg_origin
</code_context>
<issue_to_address>
**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:

```python
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:

```python
# 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:

```python
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:

```python
# 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.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

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.

[xiaoyuyu6420]

Review

Good fix for the timezone issue. The approach of reading the live next_run_time from the scheduler instead of the stale DB copy is correct, and converting to the user's configured timezone for display is the right UX.

Strengths

1. Dual lookup pattern (cron_mgr.get_next_run_time() or job.next_run_time) — handles the fire-and-forget DB write race correctly.
2. Graceful fallback when ZoneInfoNotFoundError is raised — falls back to system timezone with a warning instead of crashing.
3. Applied consistently — both add and list paths get the same timezone treatment.

Issues

1. Duplicated timezone resolution logic

The block that reads timezone from config, strips it, and creates a ZoneInfo is copy-pasted in at least two places (add and list). Per AGENTS.md's "No Unnecessary Helpers" rule, this is close to the 3+ reuse threshold. Consider extracting a small _resolve_timezone(context) -> ZoneInfo | None helper — it would also make the timezone resolution logic testable in isolation.

2. context.context.context chaining is fragile

context.context.context.get_config(...)

This triple-nested attribute access is brittle — if any layer changes its wrapping structure, this breaks silently. Is there no helper method on context to access config directly? If not, at minimum a comment explaining the layering would help.

3. Naive datetime handling assumes UTC

if next_run.tzinfo is None:
    next_run = next_run.replace(tzinfo=dt_timezone.utc)

This is correct per the comment ("Scheduler/DB values are always UTC"), but it's an assumption that should be tested. If APScheduler ever returns a naive local time (e.g. on some Windows configurations), this would produce wrong results. A test that explicitly verifies this behavior would guard against it.

4. No tests for the timezone conversion

The core value — displaying next_run_time in the user's timezone — has no test. At minimum:

• Test that a UTC next_run_time is correctly converted to a configured timezone
• Test that invalid timezone config falls back gracefully

Summary

Solid fix addressing a real UX issue. Main gaps are the duplicated timezone logic and missing tests. The context.context.context chain is worth documenting or simplifying if possible.