mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-08 05:18:49 +03:00
fix(cron): support one-time relative reminders
This commit is contained in:
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import time
|
||||||
from contextvars import ContextVar
|
from contextvars import ContextVar
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
from typing import Any
|
from typing import Any
|
||||||
@@ -28,7 +29,21 @@ _CRON_PARAMETERS = tool_parameters_schema(
|
|||||||
"(e.g., 'Send a reminder to WeChat: xxx' or 'Check system status and report'). "
|
"(e.g., 'Send a reminder to WeChat: xxx' or 'Check system status and report'). "
|
||||||
"Not used for action='list' or action='remove'."
|
"Not used for action='list' or action='remove'."
|
||||||
),
|
),
|
||||||
every_seconds=IntegerSchema(0, description="Interval in seconds (for recurring tasks)"),
|
every_seconds=IntegerSchema(
|
||||||
|
0,
|
||||||
|
description=(
|
||||||
|
"Recurring interval in seconds. Use only when the user explicitly asks to repeat "
|
||||||
|
"(e.g. 'every 20 minutes'). Do NOT use for one-time 'in/after N minutes' reminders; "
|
||||||
|
"use delay_seconds or at instead."
|
||||||
|
),
|
||||||
|
),
|
||||||
|
delay_seconds=IntegerSchema(
|
||||||
|
0,
|
||||||
|
description=(
|
||||||
|
"One-time delay in seconds for relative reminders/tasks like 'in 10 minutes' or "
|
||||||
|
"'after 1 hour'. The job runs once and is then removed."
|
||||||
|
),
|
||||||
|
),
|
||||||
cron_expr=StringSchema("Cron expression like '0 9 * * *' (for scheduled tasks)"),
|
cron_expr=StringSchema("Cron expression like '0 9 * * *' (for scheduled tasks)"),
|
||||||
tz=StringSchema(
|
tz=StringSchema(
|
||||||
"Optional IANA timezone for cron expressions (e.g. 'America/Vancouver'). "
|
"Optional IANA timezone for cron expressions (e.g. 'America/Vancouver'). "
|
||||||
@@ -46,7 +61,8 @@ _CRON_PARAMETERS = tool_parameters_schema(
|
|||||||
required=["action"],
|
required=["action"],
|
||||||
description=(
|
description=(
|
||||||
"Action-specific parameters: add requires a non-empty message plus one schedule "
|
"Action-specific parameters: add requires a non-empty message plus one schedule "
|
||||||
"(every_seconds, cron_expr, or at); remove requires job_id; list only needs action. "
|
"(every_seconds, delay_seconds, cron_expr, or at); remove requires job_id; list only "
|
||||||
|
"needs action. "
|
||||||
"Per-action requirements are enforced at runtime (see field descriptions) so the "
|
"Per-action requirements are enforced at runtime (see field descriptions) so the "
|
||||||
"top-level schema stays compatible with providers (e.g. OpenAI Codex/Responses) that "
|
"top-level schema stays compatible with providers (e.g. OpenAI Codex/Responses) that "
|
||||||
"reject oneOf/anyOf/allOf/enum/not at the root of function parameters."
|
"reject oneOf/anyOf/allOf/enum/not at the root of function parameters."
|
||||||
@@ -119,6 +135,8 @@ class CronTool(Tool, ContextAware):
|
|||||||
def description(self) -> str:
|
def description(self) -> str:
|
||||||
return (
|
return (
|
||||||
"Schedule reminders and recurring tasks. Actions: add, list, remove. "
|
"Schedule reminders and recurring tasks. Actions: add, list, remove. "
|
||||||
|
"Use delay_seconds for one-time relative requests like 'in 10 minutes'; "
|
||||||
|
"use every_seconds only for explicit repeats. "
|
||||||
f"If tz is omitted, cron expressions and naive ISO times default to {self._default_timezone}."
|
f"If tz is omitted, cron expressions and naive ISO times default to {self._default_timezone}."
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -137,6 +155,7 @@ class CronTool(Tool, ContextAware):
|
|||||||
name: str | None = None,
|
name: str | None = None,
|
||||||
message: str = "",
|
message: str = "",
|
||||||
every_seconds: int | None = None,
|
every_seconds: int | None = None,
|
||||||
|
delay_seconds: int | None = None,
|
||||||
cron_expr: str | None = None,
|
cron_expr: str | None = None,
|
||||||
tz: str | None = None,
|
tz: str | None = None,
|
||||||
at: str | None = None,
|
at: str | None = None,
|
||||||
@@ -147,7 +166,16 @@ class CronTool(Tool, ContextAware):
|
|||||||
if action == "add":
|
if action == "add":
|
||||||
if self._in_cron_context.get():
|
if self._in_cron_context.get():
|
||||||
return "Error: cannot schedule new jobs from within a cron job execution"
|
return "Error: cannot schedule new jobs from within a cron job execution"
|
||||||
return self._add_job(name, message, every_seconds, cron_expr, tz, at, deliver)
|
return self._add_job(
|
||||||
|
name,
|
||||||
|
message,
|
||||||
|
every_seconds,
|
||||||
|
cron_expr,
|
||||||
|
tz,
|
||||||
|
at,
|
||||||
|
delay_seconds=delay_seconds,
|
||||||
|
deliver=deliver,
|
||||||
|
)
|
||||||
elif action == "list":
|
elif action == "list":
|
||||||
return self._list_jobs()
|
return self._list_jobs()
|
||||||
elif action == "remove":
|
elif action == "remove":
|
||||||
@@ -162,6 +190,7 @@ class CronTool(Tool, ContextAware):
|
|||||||
cron_expr: str | None,
|
cron_expr: str | None,
|
||||||
tz: str | None,
|
tz: str | None,
|
||||||
at: str | None,
|
at: str | None,
|
||||||
|
delay_seconds: int | None = None,
|
||||||
deliver: bool = True,
|
deliver: bool = True,
|
||||||
) -> str:
|
) -> str:
|
||||||
if not message:
|
if not message:
|
||||||
@@ -180,10 +209,22 @@ class CronTool(Tool, ContextAware):
|
|||||||
if err := self._validate_timezone(tz):
|
if err := self._validate_timezone(tz):
|
||||||
return err
|
return err
|
||||||
|
|
||||||
|
schedule_params = [
|
||||||
|
every_seconds is not None and every_seconds > 0,
|
||||||
|
delay_seconds is not None and delay_seconds > 0,
|
||||||
|
bool(cron_expr),
|
||||||
|
bool(at),
|
||||||
|
]
|
||||||
|
if sum(schedule_params) > 1:
|
||||||
|
return "Error: use exactly one of every_seconds, delay_seconds, cron_expr, or at"
|
||||||
|
|
||||||
# Build schedule
|
# Build schedule
|
||||||
delete_after = False
|
delete_after = False
|
||||||
if every_seconds:
|
if every_seconds:
|
||||||
schedule = CronSchedule(kind="every", every_ms=every_seconds * 1000)
|
schedule = CronSchedule(kind="every", every_ms=every_seconds * 1000)
|
||||||
|
elif delay_seconds:
|
||||||
|
schedule = CronSchedule(kind="at", at_ms=int(time.time() * 1000) + delay_seconds * 1000)
|
||||||
|
delete_after = True
|
||||||
elif cron_expr:
|
elif cron_expr:
|
||||||
effective_tz = tz or self._default_timezone
|
effective_tz = tz or self._default_timezone
|
||||||
if err := self._validate_timezone(effective_tz):
|
if err := self._validate_timezone(effective_tz):
|
||||||
|
|||||||
@@ -15,7 +15,7 @@ Use the `cron` tool to schedule reminders or recurring tasks.
|
|||||||
|
|
||||||
## Examples
|
## Examples
|
||||||
|
|
||||||
Fixed reminder:
|
Recurring reminder:
|
||||||
```
|
```
|
||||||
cron(action="add", message="Time to take a break!", every_seconds=1200)
|
cron(action="add", message="Time to take a break!", every_seconds=1200)
|
||||||
```
|
```
|
||||||
@@ -30,6 +30,11 @@ One-time scheduled task (compute ISO datetime from current time):
|
|||||||
cron(action="add", message="Remind me about the meeting", at="<ISO datetime>")
|
cron(action="add", message="Remind me about the meeting", at="<ISO datetime>")
|
||||||
```
|
```
|
||||||
|
|
||||||
|
One-time relative reminder:
|
||||||
|
```
|
||||||
|
cron(action="add", message="Remind me to drink water", delay_seconds=60)
|
||||||
|
```
|
||||||
|
|
||||||
Timezone-aware cron:
|
Timezone-aware cron:
|
||||||
```
|
```
|
||||||
cron(action="add", message="Morning standup", cron_expr="0 9 * * 1-5", tz="America/Vancouver")
|
cron(action="add", message="Morning standup", cron_expr="0 9 * * 1-5", tz="America/Vancouver")
|
||||||
@@ -45,8 +50,10 @@ cron(action="remove", job_id="abc123")
|
|||||||
|
|
||||||
| User says | Parameters |
|
| User says | Parameters |
|
||||||
|-----------|------------|
|
|-----------|------------|
|
||||||
|
| in 20 minutes / after 20 minutes | delay_seconds: 1200 |
|
||||||
|
| one hour later | delay_seconds: 3600 |
|
||||||
| every 20 minutes | every_seconds: 1200 |
|
| every 20 minutes | every_seconds: 1200 |
|
||||||
| every hour | every_seconds: 3600 |
|
| every hour / repeat hourly | every_seconds: 3600 |
|
||||||
| every day at 8am | cron_expr: "0 8 * * *" |
|
| every day at 8am | cron_expr: "0 8 * * *" |
|
||||||
| weekdays at 5pm | cron_expr: "0 17 * * 1-5" |
|
| weekdays at 5pm | cron_expr: "0 17 * * 1-5" |
|
||||||
| 9am Vancouver time daily | cron_expr: "0 9 * * *", tz: "America/Vancouver" |
|
| 9am Vancouver time daily | cron_expr: "0 9 * * *", tz: "America/Vancouver" |
|
||||||
|
|||||||
@@ -137,6 +137,21 @@ async def test_run_history_records_errors(tmp_path) -> None:
|
|||||||
assert loaded.state.run_history[0].error == "boom"
|
assert loaded.state.run_history[0].error == "boom"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_one_shot_job_deletes_after_run(tmp_path) -> None:
|
||||||
|
store_path = tmp_path / "cron" / "jobs.json"
|
||||||
|
service = CronService(store_path, on_job=lambda _: asyncio.sleep(0))
|
||||||
|
job = service.add_job(
|
||||||
|
name="one-shot",
|
||||||
|
schedule=CronSchedule(kind="at", at_ms=int(time.time() * 1000) + 60_000),
|
||||||
|
message="hello",
|
||||||
|
delete_after_run=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert await service.run_job(job.id, force=True) is True
|
||||||
|
assert service.get_job(job.id) is None
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_run_history_trimmed_to_max(tmp_path) -> None:
|
async def test_run_history_trimmed_to_max(tmp_path) -> None:
|
||||||
store_path = tmp_path / "cron" / "jobs.json"
|
store_path = tmp_path / "cron" / "jobs.json"
|
||||||
|
|||||||
@@ -324,6 +324,47 @@ def test_add_at_job_uses_default_timezone_for_naive_datetime(tmp_path) -> None:
|
|||||||
assert job.schedule.at_ms == expected
|
assert job.schedule.at_ms == expected
|
||||||
|
|
||||||
|
|
||||||
|
def test_add_delay_job_creates_one_shot_at_schedule(tmp_path) -> None:
|
||||||
|
tool = _make_tool(tmp_path)
|
||||||
|
tool.set_context(RequestContext(channel="telegram", chat_id="chat-1"))
|
||||||
|
before = int(datetime.now(tz=timezone.utc).timestamp() * 1000)
|
||||||
|
|
||||||
|
result = tool._add_job(
|
||||||
|
None,
|
||||||
|
"Drink water",
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
delay_seconds=60,
|
||||||
|
)
|
||||||
|
|
||||||
|
after = int(datetime.now(tz=timezone.utc).timestamp() * 1000)
|
||||||
|
assert result.startswith("Created job")
|
||||||
|
job = tool._cron.list_jobs()[0]
|
||||||
|
assert job.schedule.kind == "at"
|
||||||
|
assert job.delete_after_run is True
|
||||||
|
assert before + 60_000 <= (job.schedule.at_ms or 0) <= after + 60_000
|
||||||
|
|
||||||
|
|
||||||
|
def test_add_job_rejects_multiple_schedule_modes(tmp_path) -> None:
|
||||||
|
tool = _make_tool(tmp_path)
|
||||||
|
tool.set_context(RequestContext(channel="telegram", chat_id="chat-1"))
|
||||||
|
|
||||||
|
result = tool._add_job(
|
||||||
|
None,
|
||||||
|
"Drink water",
|
||||||
|
60,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
delay_seconds=60,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert "exactly one" in result
|
||||||
|
assert tool._cron.list_jobs() == []
|
||||||
|
|
||||||
|
|
||||||
def test_add_job_delivers_by_default(tmp_path) -> None:
|
def test_add_job_delivers_by_default(tmp_path) -> None:
|
||||||
tool = _make_tool(tmp_path)
|
tool = _make_tool(tmp_path)
|
||||||
tool.set_context(RequestContext(channel="telegram", chat_id="chat-1"))
|
tool.set_context(RequestContext(channel="telegram", chat_id="chat-1"))
|
||||||
|
|||||||
@@ -87,6 +87,14 @@ class TestSchemaSelfDescribesRequirements:
|
|||||||
desc = tool.parameters["properties"]["message"]["description"]
|
desc = tool.parameters["properties"]["message"]["description"]
|
||||||
assert "REQUIRED" in desc and "action='add'" in desc
|
assert "REQUIRED" in desc and "action='add'" in desc
|
||||||
|
|
||||||
|
def test_delay_description_flags_relative_one_time_reminders(self) -> None:
|
||||||
|
tool = CronTool(_SvcStub())
|
||||||
|
delay_desc = tool.parameters["properties"]["delay_seconds"]["description"]
|
||||||
|
every_desc = tool.parameters["properties"]["every_seconds"]["description"]
|
||||||
|
assert "One-time" in delay_desc
|
||||||
|
assert "in/after" in every_desc
|
||||||
|
assert "Do NOT use" in every_desc
|
||||||
|
|
||||||
def test_job_id_description_flags_remove_requirement(self) -> None:
|
def test_job_id_description_flags_remove_requirement(self) -> None:
|
||||||
tool = CronTool(_SvcStub())
|
tool = CronTool(_SvcStub())
|
||||||
desc = tool.parameters["properties"]["job_id"]["description"]
|
desc = tool.parameters["properties"]["job_id"]["description"]
|
||||||
|
|||||||
Reference in New Issue
Block a user