fix(cron): accept documented "every <weekday> <time>" schedules
parse_schedule's "every " branch passed everything after the prefix straight to parse_duration(), so documented natural-language schedules like "every monday 9am" and "every day at 9am" (AGENTS.md, SKILL.md, cron docs) were rejected with "Invalid duration". Convert weekday and daily/weekday/weekend phrases to cron expressions before the duration fallback; "every 30m"/"every 2h" interval parsing is unchanged.
This commit is contained in:
+113
-5
@@ -794,11 +794,99 @@ def parse_duration(s: str) -> int:
|
||||
|
||||
value = int(match.group(1)) if match.group(1) else 1
|
||||
unit = match.group(2)[0] # First char: m, h, or d
|
||||
|
||||
|
||||
multipliers = {'m': 1, 'h': 60, 'd': 1440}
|
||||
return value * multipliers[unit]
|
||||
|
||||
|
||||
# Natural-language day-spec phrases for the documented "every monday 9am" /
|
||||
# "every day at 9am" schedule forms. Cron weekday numbering is
|
||||
# 0=Sunday … 6=Saturday (croniter's default).
|
||||
_WEEKDAY_TO_CRON_DOW = {
|
||||
"sunday": "0", "sun": "0",
|
||||
"monday": "1", "mon": "1",
|
||||
"tuesday": "2", "tue": "2", "tues": "2",
|
||||
"wednesday": "3", "wed": "3", "weds": "3",
|
||||
"thursday": "4", "thu": "4", "thur": "4", "thurs": "4",
|
||||
"friday": "5", "fri": "5",
|
||||
"saturday": "6", "sat": "6",
|
||||
}
|
||||
|
||||
# Keyword day-specs that expand to a cron weekday field.
|
||||
_DAYSPEC_TO_CRON_DOW = {
|
||||
"day": "*", "daily": "*", "everyday": "*",
|
||||
"weekday": "1-5", "weekdays": "1-5",
|
||||
"weekend": "0,6", "weekends": "0,6",
|
||||
}
|
||||
|
||||
|
||||
def _parse_clock_time(text: str) -> Optional[tuple]:
|
||||
"""Parse a wall-clock time into a ``(hour, minute)`` 24-hour tuple.
|
||||
|
||||
Accepts ``9am``, ``9:30am``, ``9 am``, ``14:00``, ``7`` (bare hour, 24h),
|
||||
``noon``/``midday``, and ``midnight``. Returns None when the text is not a
|
||||
recognized clock time so the caller can reject the schedule cleanly.
|
||||
"""
|
||||
t = text.strip().lower().replace(" ", "")
|
||||
if not t:
|
||||
return None
|
||||
if t in ("noon", "midday"):
|
||||
return (12, 0)
|
||||
if t == "midnight":
|
||||
return (0, 0)
|
||||
match = re.match(r'^(\d{1,2})(?::(\d{2}))?(am|pm)?$', t)
|
||||
if not match:
|
||||
return None
|
||||
hour = int(match.group(1))
|
||||
minute = int(match.group(2) or 0)
|
||||
meridiem = match.group(3)
|
||||
if meridiem:
|
||||
if not 1 <= hour <= 12:
|
||||
return None
|
||||
if meridiem == "am":
|
||||
hour = 0 if hour == 12 else hour
|
||||
else: # pm
|
||||
hour = 12 if hour == 12 else hour + 12
|
||||
if hour > 23 or minute > 59:
|
||||
return None
|
||||
return (hour, minute)
|
||||
|
||||
|
||||
def _natural_every_to_cron(rest: str) -> Optional[str]:
|
||||
"""Convert a documented ``every <when> [at] <time>`` phrase to a 5-field
|
||||
cron expression, or None when *rest* is not such a phrase.
|
||||
|
||||
Examples::
|
||||
|
||||
"monday 9am" -> "0 9 * * 1"
|
||||
"day at 9am" -> "0 9 * * *"
|
||||
"weekday at 9am" -> "0 9 * * 1-5"
|
||||
|
||||
Returning None lets ``parse_schedule`` fall back to the interval
|
||||
(``every 30m``) path, so existing duration schedules are unaffected.
|
||||
"""
|
||||
tokens = rest.lower().split()
|
||||
if not tokens:
|
||||
return None
|
||||
day_token = tokens[0]
|
||||
dow = _WEEKDAY_TO_CRON_DOW.get(day_token) or _DAYSPEC_TO_CRON_DOW.get(day_token)
|
||||
if dow is None:
|
||||
return None
|
||||
|
||||
time_tokens = tokens[1:]
|
||||
# Optional "at" separator: "every day at 9am".
|
||||
if time_tokens and time_tokens[0] == "at":
|
||||
time_tokens = time_tokens[1:]
|
||||
if not time_tokens:
|
||||
return None
|
||||
|
||||
parsed = _parse_clock_time(" ".join(time_tokens))
|
||||
if parsed is None:
|
||||
return None
|
||||
hour, minute = parsed
|
||||
return f"{minute} {hour} * * {dow}"
|
||||
|
||||
|
||||
def parse_schedule(schedule: str) -> Dict[str, Any]:
|
||||
"""
|
||||
Parse schedule string into structured format.
|
||||
@@ -814,17 +902,36 @@ def parse_schedule(schedule: str) -> Dict[str, Any]:
|
||||
"2h" → once in 2 hours
|
||||
"every 30m" → recurring every 30 minutes
|
||||
"every 2h" → recurring every 2 hours
|
||||
"every monday 9am" → recurring weekly (cron)
|
||||
"every day at 9am" → recurring daily (cron)
|
||||
"0 9 * * *" → cron expression
|
||||
"2026-02-03T14:00" → once at timestamp
|
||||
"""
|
||||
schedule = schedule.strip()
|
||||
original = schedule
|
||||
schedule_lower = schedule.lower()
|
||||
|
||||
# "every X" pattern → recurring interval
|
||||
|
||||
# "every X" pattern → recurring interval, OR a documented natural-language
|
||||
# day/time phrase ("every monday 9am", "every day at 9am") → cron.
|
||||
if schedule_lower.startswith("every "):
|
||||
duration_str = schedule[6:].strip()
|
||||
minutes = parse_duration(duration_str)
|
||||
rest = schedule[6:].strip()
|
||||
cron_expr = _natural_every_to_cron(rest)
|
||||
if cron_expr is not None:
|
||||
if not HAS_CRONITER:
|
||||
raise ValueError(
|
||||
"Weekday/time schedules like 'every monday 9am' require the "
|
||||
"'croniter' package. Install with: pip install croniter"
|
||||
)
|
||||
try:
|
||||
croniter(cron_expr)
|
||||
except Exception as e:
|
||||
raise ValueError(f"Invalid schedule '{original}': {e}")
|
||||
return {
|
||||
"kind": "cron",
|
||||
"expr": cron_expr,
|
||||
"display": original,
|
||||
}
|
||||
minutes = parse_duration(rest)
|
||||
return {
|
||||
"kind": "interval",
|
||||
"minutes": minutes,
|
||||
@@ -894,6 +1001,7 @@ def parse_schedule(schedule: str) -> Dict[str, Any]:
|
||||
f"Invalid schedule '{original}'. Use:\n"
|
||||
f" - Duration: '30m', '2h', '1d' (one-shot)\n"
|
||||
f" - Interval: 'every 30m', 'every 2h' (recurring)\n"
|
||||
f" - Weekly/daily: 'every monday 9am', 'every day at 9am' (recurring)\n"
|
||||
f" - Cron: '0 9 * * *' (cron expression)\n"
|
||||
f" - Timestamp: '2026-02-03T14:00:00' (one-shot at time)"
|
||||
)
|
||||
|
||||
@@ -94,6 +94,86 @@ class TestParseSchedule:
|
||||
assert result["minutes"] == 120
|
||||
|
||||
|
||||
# ---- Natural-language weekday/daily phrases → cron (issue: documented
|
||||
# "every monday 9am" format was rejected because the "every" branch only
|
||||
# accepted durations). ----
|
||||
|
||||
def test_every_weekday_time_becomes_cron(self):
|
||||
pytest.importorskip("croniter")
|
||||
result = parse_schedule("every monday 9am")
|
||||
assert result["kind"] == "cron"
|
||||
assert result["expr"] == "0 9 * * 1"
|
||||
# Display preserves the user's natural phrasing.
|
||||
assert result["display"] == "every monday 9am"
|
||||
|
||||
def test_every_sunday_maps_to_zero(self):
|
||||
pytest.importorskip("croniter")
|
||||
# Cron weekday numbering puts Sunday at 0.
|
||||
assert parse_schedule("every sunday 9am")["expr"] == "0 9 * * 0"
|
||||
|
||||
def test_every_weekday_abbreviations(self):
|
||||
pytest.importorskip("croniter")
|
||||
assert parse_schedule("every mon 9am")["expr"] == "0 9 * * 1"
|
||||
assert parse_schedule("every fri 5pm")["expr"] == "0 17 * * 5"
|
||||
|
||||
def test_every_day_keyword_is_daily(self):
|
||||
pytest.importorskip("croniter")
|
||||
assert parse_schedule("every day at 9am")["expr"] == "0 9 * * *"
|
||||
assert parse_schedule("every day 7am")["expr"] == "0 7 * * *"
|
||||
|
||||
def test_every_weekday_keyword_is_business_days(self):
|
||||
pytest.importorskip("croniter")
|
||||
assert parse_schedule("every weekday at 9am")["expr"] == "0 9 * * 1-5"
|
||||
|
||||
def test_every_weekend_keyword(self):
|
||||
pytest.importorskip("croniter")
|
||||
assert parse_schedule("every weekend at 10am")["expr"] == "0 10 * * 0,6"
|
||||
|
||||
def test_every_time_formats(self):
|
||||
pytest.importorskip("croniter")
|
||||
# 24-hour, explicit minutes, noon/midnight, bare hour.
|
||||
assert parse_schedule("every monday 14:30")["expr"] == "30 14 * * 1"
|
||||
assert parse_schedule("every monday 9:05am")["expr"] == "5 9 * * 1"
|
||||
assert parse_schedule("every monday noon")["expr"] == "0 12 * * 1"
|
||||
assert parse_schedule("every monday midnight")["expr"] == "0 0 * * 1"
|
||||
assert parse_schedule("every monday at 7")["expr"] == "0 7 * * 1"
|
||||
|
||||
def test_every_12_hour_boundaries(self):
|
||||
pytest.importorskip("croniter")
|
||||
# 12am is midnight (00:00), 12pm is noon (12:00).
|
||||
assert parse_schedule("every monday 12am")["expr"] == "0 0 * * 1"
|
||||
assert parse_schedule("every monday 12pm")["expr"] == "0 12 * * 1"
|
||||
|
||||
def test_every_weekday_time_is_case_insensitive(self):
|
||||
pytest.importorskip("croniter")
|
||||
assert parse_schedule("Every Monday 9AM")["expr"] == "0 9 * * 1"
|
||||
|
||||
def test_every_weekday_schedule_computes_next_run(self):
|
||||
pytest.importorskip("croniter")
|
||||
# End-to-end: the produced cron schedule is usable by compute_next_run.
|
||||
schedule = parse_schedule("every monday 9am")
|
||||
next_run = compute_next_run(schedule)
|
||||
assert next_run is not None
|
||||
dt = datetime.fromisoformat(next_run)
|
||||
assert dt.weekday() == 0 # Python: Monday == 0
|
||||
assert (dt.hour, dt.minute) == (9, 0)
|
||||
|
||||
def test_every_duration_still_interval(self):
|
||||
# The interval path must keep working unchanged.
|
||||
assert parse_schedule("every 30m")["kind"] == "interval"
|
||||
assert parse_schedule("every 1d")["minutes"] == 1440
|
||||
|
||||
def test_every_weekday_without_time_raises(self):
|
||||
# A weekday with no time is ambiguous — reject rather than guess.
|
||||
with pytest.raises(ValueError):
|
||||
parse_schedule("every monday")
|
||||
|
||||
def test_every_invalid_time_raises(self):
|
||||
with pytest.raises(ValueError):
|
||||
parse_schedule("every monday 25am")
|
||||
with pytest.raises(ValueError):
|
||||
parse_schedule("every monday 9pm pizza")
|
||||
|
||||
def test_cron_expression(self):
|
||||
pytest.importorskip("croniter")
|
||||
result = parse_schedule("0 9 * * *")
|
||||
|
||||
@@ -246,6 +246,44 @@ class TestUnifiedCronjobTool:
|
||||
assert listing["jobs"][0]["name"] == "Server Check"
|
||||
assert listing["jobs"][0]["state"] == "scheduled"
|
||||
|
||||
def test_create_with_natural_weekday_schedule(self):
|
||||
# The documented "every monday 9am" form must create a real cron job
|
||||
# through the tool path, not error out (issue: parser rejected it).
|
||||
pytest.importorskip("croniter")
|
||||
created = json.loads(
|
||||
cronjob(
|
||||
action="create",
|
||||
prompt="Weekly report",
|
||||
schedule="every monday 9am",
|
||||
name="Weekly Report",
|
||||
)
|
||||
)
|
||||
assert created["success"] is True
|
||||
# Display keeps the user's natural phrasing.
|
||||
assert created["schedule"] == "every monday 9am"
|
||||
# A recurring job must have a computed next run.
|
||||
assert created["next_run_at"]
|
||||
|
||||
# The stored schedule is a cron expression.
|
||||
from cron.jobs import get_job
|
||||
stored = get_job(created["job_id"])
|
||||
assert stored["schedule"]["kind"] == "cron"
|
||||
assert stored["schedule"]["expr"] == "0 9 * * 1"
|
||||
|
||||
def test_update_to_natural_weekday_schedule(self):
|
||||
pytest.importorskip("croniter")
|
||||
created = json.loads(cronjob(action="create", prompt="Check", schedule="every 1h"))
|
||||
job_id = created["job_id"]
|
||||
|
||||
updated = json.loads(
|
||||
cronjob(action="update", job_id=job_id, schedule="every day at 9am")
|
||||
)
|
||||
assert updated["success"] is True
|
||||
assert updated["job"]["schedule"] == "every day at 9am"
|
||||
|
||||
from cron.jobs import get_job
|
||||
assert get_job(job_id)["schedule"]["expr"] == "0 9 * * *"
|
||||
|
||||
def test_list_handles_partial_legacy_job_records(self):
|
||||
from cron.jobs import save_jobs
|
||||
|
||||
|
||||
@@ -1969,8 +1969,7 @@ Jobs run in a fresh session with no current-chat context, so prompts must be sel
|
||||
"description": "For create: the full self-contained prompt (paired with any skills as the task instruction). For run: optional transient context for that single fire (never persisted)."
|
||||
},
|
||||
"schedule": {
|
||||
"type": "string",
|
||||
"description": "REQUIRED for create. '30m' (every 30 minutes), 'every 2h', cron syntax '0 9 * * *' (daily 9am), or an ISO timestamp for one-shot ('2026-06-01T09:00:00')."
|
||||
"t "description": "REQUIRED for create. '30m' (every 30 minutes), 'every 2h', 'every monday 9am' / 'every day at 9am' (recurring weekly/daily), cron syntax '0 9 * * *' (daily 9am), or an ISO timestamp for one-shot ('2026-06-01T09:00:00')."
|
||||
},
|
||||
"name": {
|
||||
"type": "string",
|
||||
|
||||
Reference in New Issue
Block a user