From e8bab87ba79da978d478a51c76f368554fb03418 Mon Sep 17 00:00:00 2001 From: andrexibiza <84248988+andrexibiza@users.noreply.github.com> Date: Tue, 4 Aug 2026 17:04:09 -0500 Subject: [PATCH] fix(cron): bare durations are recurring intervals; coerce repeat string forms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contract bug (2026-08-04): the cronjob tool schema documents '30m' as '(every 30 minutes)' — recurring — but parse_schedule returned kind='once' for bare durations, silently creating a one-shot job for a recurring request (agent passed '30m' for 'every 30 min', job ran once and died). Bare durations ('30m','2h','1d') now parse as recurring intervals matching the documented contract; explicit one-shot by duration is 'in 30m'/'in 2h' (fires once that far from now). ISO timestamps stay one-shot. Also fixes the sibling repeat-coercion class (#66824/#64520/#7142): repeat='forever'/'once'/'N' strings now coerce in create_job instead of raising "'<=' not supported between instances of 'str' and 'int'". Tool description rewritten to teach the corrected contract and steer relative requests to 'in Nm' (no more hand-computed ISO timestamps). Supersedes the doc-only direction of #53739 while keeping its goal (relative one-shots must be expressible) via the 'in X' form. Signed-off-by: andrexibiza <84248988+andrexibiza@users.noreply.github.com> --- cron/jobs.py | 53 ++++++++++++++++++++++++++++++++------ tests/cron/test_jobs.py | 56 +++++++++++++++++++++++++++++++---------- tools/cronjob_tools.py | 3 ++- 3 files changed, 91 insertions(+), 21 deletions(-) diff --git a/cron/jobs.py b/cron/jobs.py index 3c55f01d40..5d6a1bdaf7 100644 --- a/cron/jobs.py +++ b/cron/jobs.py @@ -919,8 +919,8 @@ def parse_schedule(schedule: str) -> Dict[str, Any]: - For "cron": "expr" (cron expression) Examples: - "30m" → once in 30 minutes - "2h" → once in 2 hours + "30m" → every 30 minutes (recurring) + "2h" → every 2 hours (recurring) "every 30m" → recurring every 30 minutes "every 2h" → recurring every 2 hours "every monday 9am" → recurring weekly (cron) @@ -1030,14 +1030,32 @@ def parse_schedule(schedule: str) -> Dict[str, Any]: except ValueError as e: raise ValueError(f"Invalid timestamp '{schedule}': {e}") - # Duration like "30m", "2h", "1d" → one-shot from now - try: - minutes = parse_duration(schedule) + # Duration like "30m", "2h", "1d" → RECURRING interval, matching the + # documented tool contract ("30m (every 30 minutes)"). Previously this + # returned kind="once", silently creating a one-shot job for a schedule + # the schema documents as recurring — an agent passing '30m' for "every + # 30 minutes" got a job that ran once and died (cron contract bug, fixed + # 2026-08-04). Explicit one-shot-by-duration is "in 30m"/"in 2h". + if schedule_lower.startswith("in "): + duration_str = schedule[3:].strip() + try: + minutes = parse_duration(duration_str) + except ValueError: + raise ValueError( + f"Invalid duration '{duration_str}' after 'in '. Use e.g. 'in 30m', 'in 2h'." + ) run_at = _hermes_now() + timedelta(minutes=minutes) return { "kind": "once", "run_at": run_at.isoformat(), - "display": f"once in {original}" + "display": f"once in {duration_str}", + } + try: + minutes = parse_duration(schedule) + return { + "kind": "interval", + "minutes": minutes, + "display": f"every {minutes}m", } except ValueError: pass @@ -2229,7 +2247,28 @@ def create_job( """ parsed_schedule = parse_schedule(schedule) - # Normalize repeat: treat 0 or negative values as None (infinite) + # Normalize repeat: treat 0 or negative values as None (infinite). + # Also coerce the documented string forms ('forever' -> None, + # 'once' -> 1, numeric strings -> int) — the tool schema exposes repeat + # as an integer but agents legitimately pass the user-facing strings + # 'forever'/'once', which previously died with + # "'<=' not supported between instances of 'str' and 'int'" + # (#66824/#64520/#7142). Coerce here so every entry point (tool, CLI, + # API, dashboard) inherits the fix. + if isinstance(repeat, str): + repeat_str = repeat.strip().lower() + if repeat_str in ("forever", "infinite", "inf", "none", ""): + repeat = None + elif repeat_str in ("once", "one", "1x"): + repeat = 1 + else: + try: + repeat = int(repeat_str) + except ValueError: + raise ValueError( + f"Invalid repeat value {repeat!r}: use an integer, " + f"'forever', or 'once'." + ) if repeat is not None and repeat <= 0: repeat = None diff --git a/tests/cron/test_jobs.py b/tests/cron/test_jobs.py index 4dffbcc6f2..4218c52cbc 100644 --- a/tests/cron/test_jobs.py +++ b/tests/cron/test_jobs.py @@ -76,11 +76,24 @@ class TestParseDuration: # ========================================================================= class TestParseSchedule: - def test_duration_becomes_once(self): + def test_bare_duration_becomes_recurring_interval(self): + """Contract: bare '30m' means EVERY 30 minutes (tool schema says so). + + Regression for the cron contract bug (2026-08-04): parse_schedule + returned kind='once' for bare durations, so an agent passing '30m' + for 'every 30 minutes' silently got a one-shot job that ran once and + died. The documented tool contract (tools/cronjob_tools.py) says + '30m' (every 30 minutes) — the code now honors it. + """ result = parse_schedule("30m") + assert result["kind"] == "interval" + assert result["minutes"] == 30 + assert "run_at" not in result + + def test_in_duration_becomes_once(self): + """Explicit one-shot by duration: 'in 30m' fires once in 30 minutes.""" + result = parse_schedule("in 30m") assert result["kind"] == "once" - assert "run_at" in result - # run_at should be a valid ISO timestamp string ~30 minutes from now run_at_str = result["run_at"] assert isinstance(run_at_str, str) run_at = datetime.fromisoformat(run_at_str) @@ -359,7 +372,7 @@ class TestJobCRUD: assert job["id"] assert job["prompt"] == "Check server status" assert job["enabled"] is True - assert job["schedule"]["kind"] == "once" + assert job["schedule"]["kind"] == "interval" fetched = get_job(job["id"]) assert fetched is not None @@ -379,9 +392,26 @@ class TestJobCRUD: def test_auto_repeat_for_once(self, tmp_cron_dir): - job = create_job(prompt="One-shot", schedule="1h") + job = create_job(prompt="One-shot", schedule="in 1h") assert job["repeat"]["times"] == 1 + def test_repeat_string_forms_coerced(self, tmp_cron_dir): + """Agents pass 'forever'/'once' as repeat — must coerce, not TypeError. + + Regression for #66824/#64520/#7142: repeat='forever' died with + "'<=' not supported between instances of 'str' and 'int'". The tool + schema documents repeat as an integer but user-facing forms are + strings; coerce at create_job so every entry point inherits it. + """ + forever = create_job(prompt="Str forever", schedule="every 1h", repeat="forever") + assert forever["repeat"]["times"] is None # None = infinite + once = create_job(prompt="Str once", schedule="every 1h", repeat="once") + assert once["repeat"]["times"] == 1 + three = create_job(prompt="Str 3", schedule="every 1h", repeat="3") + assert three["repeat"]["times"] == 3 + with pytest.raises(ValueError, match="Invalid repeat"): + create_job(prompt="Bad", schedule="every 1h", repeat="banana") + def test_rejects_stale_past_one_shot_at_creation(self, tmp_cron_dir, monkeypatch): now = datetime(2026, 3, 18, 4, 30, 0, tzinfo=timezone.utc) monkeypatch.setattr("cron.jobs._hermes_now", lambda: now) @@ -569,7 +599,7 @@ class TestMarkJobRun: def test_repeat_limit_retains_completed_record(self, tmp_cron_dir): """A finished one-shot must stay inspectable, not vanish from the store.""" - job = create_job(prompt="Once", schedule="30m", repeat=1) + job = create_job(prompt="Once", schedule="in 30m", repeat=1) mark_job_run(job["id"], success=True) updated = get_job(job["id"]) assert updated is not None, "completed one-shot was deleted from jobs.json" @@ -580,7 +610,7 @@ class TestMarkJobRun: def test_repeat_limit_retains_delivery_error(self, tmp_cron_dir): """A one-shot whose delivery failed must keep the error on its record.""" - job = create_job(prompt="Once", schedule="30m", repeat=1) + job = create_job(prompt="Once", schedule="in 30m", repeat=1) mark_job_run( job["id"], success=True, delivery_error="platform 'telegram' not configured", @@ -592,7 +622,7 @@ class TestMarkJobRun: def test_completed_oneshot_visible_in_list(self, tmp_cron_dir): """list_jobs(include_disabled=True) surfaces the completed record.""" - job = create_job(prompt="Once", schedule="30m", repeat=1) + job = create_job(prompt="Once", schedule="in 30m", repeat=1) mark_job_run(job["id"], success=True, delivery_error="send failed: 502") listed = {j["id"]: j for j in list_jobs(include_disabled=True)} assert job["id"] in listed @@ -603,7 +633,7 @@ class TestMarkJobRun: def test_completed_oneshot_not_due(self, tmp_cron_dir): """A retained completed one-shot must never be dispatched again.""" - job = create_job(prompt="Once", schedule="30m", repeat=1) + job = create_job(prompt="Once", schedule="in 30m", repeat=1) mark_job_run(job["id"], success=True) assert job["id"] not in {j["id"] for j in get_due_jobs()} @@ -708,7 +738,7 @@ class TestAdvanceNextRun: def test_skips_oneshot_job(self, tmp_cron_dir): """One-shot jobs should NOT be advanced — they need to retry on restart.""" - job = create_job(prompt="Run once", schedule="30m") + job = create_job(prompt="Run once", schedule="in 30m") original_next = get_job(job["id"])["next_run_at"] result = advance_next_run(job["id"]) @@ -1166,7 +1196,7 @@ class TestCronOutputRetention: d.mkdir(parents=True, exist_ok=True) names = [f"2026-06-25_10-00-{i:02d}.md" for i in range(count)] for n in names: - (d / n).write_text("x") + (d / n).write_text("x", encoding="utf-8") return names def test_prune_keeps_newest_n(self, tmp_path): @@ -1646,7 +1676,7 @@ class TestAdvanceNextRuns: def _make_due(self, tmp_cron_dir, n_recurring=3, n_oneshot=1): rec = [create_job(prompt=f"rec {i}", schedule="every 1h") for i in range(n_recurring)] - one = [create_job(prompt=f"one {i}", schedule="30m") + one = [create_job(prompt=f"one {i}", schedule="in 30m") for i in range(n_oneshot)] jobs = load_jobs() old = (datetime.now() - timedelta(minutes=5)).isoformat() @@ -1712,7 +1742,7 @@ class TestCompletedOneshotRetentionSweep: def _completed_oneshot(self, age_days: float): """Create a one-shot, complete it, and backdate its last_run_at.""" - job = create_job(prompt="Once", schedule="30m", repeat=1) + job = create_job(prompt="Once", schedule="in 30m", repeat=1) mark_job_run(job["id"], success=True, delivery_error="boom") stamp = ( datetime.now(timezone.utc) - timedelta(days=age_days) diff --git a/tools/cronjob_tools.py b/tools/cronjob_tools.py index 019dce96d8..5994220d62 100644 --- a/tools/cronjob_tools.py +++ b/tools/cronjob_tools.py @@ -1970,7 +1970,8 @@ Jobs run in a fresh session with no current-chat context, so prompts must be sel }, "schedule": { "type": "string", - "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')." + "type": "string", + "description": "REQUIRED for create. Schedule forms: (1) recurring interval — '30m', 'every 2h', 'every hour' (EVERY 30 minutes / 2 hours / hour, forever by default); (2) explicit one-shot by duration — 'in 30m', 'in 2h' (fires ONCE that far from now; use this for 'remind me in N minutes' — do NOT hand-compute an absolute timestamp); (3) natural day/time — 'every monday 9am', 'weekdays at 9am', 'every day at 9am' (recurring weekly/daily); (4) cron syntax — '0 9 * * *' (daily 9am); (5) absolute one-shot — ISO timestamp '2026-06-01T09:00:00'." }, "name": { "type": "string",