diff --git a/cron/jobs.py b/cron/jobs.py index f623dad7c0..42b339c6cd 100644 --- a/cron/jobs.py +++ b/cron/jobs.py @@ -774,6 +774,36 @@ def ensure_dirs(): # Schedule Parsing # ============================================================================= +def normalize_repeat_value(repeat: Any) -> Optional[int]: + """Coerce a repeat value from any entry point into ``Optional[int]``. + + The tool schema exposes ``repeat`` as an integer, but agents and users + legitimately pass the user-facing strings ``'forever'``/``'once'`` or + numeric strings (``'3'``). Uncoerced strings previously died with + ``'<=' not supported between instances of 'str' and 'int'`` at create + (#66824/#64520/#7142/#71987/#95706) and were stored raw by update paths, + breaking ``mark_job_run`` later. Semantics: ``'forever'``-family -> None + (infinite), ``'once'``-family -> 1, numeric -> int, 0/negative -> None, + anything else -> ValueError (never store garbage). + """ + if repeat is None: + return None + if isinstance(repeat, str): + repeat_str = repeat.strip().lower() + if repeat_str in ("forever", "infinite", "inf", "none", ""): + return None + if repeat_str in ("once", "one", "1x"): + return 1 + try: + repeat = int(repeat_str) + except ValueError: + raise ValueError( + f"Invalid repeat value {repeat!r}: use an integer, " + f"'forever', or 'once'." + ) + return None if repeat <= 0 else int(repeat) + + def parse_duration(s: str) -> int: """ Parse duration string into minutes. @@ -2248,29 +2278,10 @@ def create_job( parsed_schedule = parse_schedule(schedule) # 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 + # String forms ('forever'/'once'/numeric) coerce via + # normalize_repeat_value — the shared chokepoint with update paths + # (#66824/#64520/#7142/#71987/#95706). + repeat = normalize_repeat_value(repeat) # Auto-set repeat=1 for one-shot schedules if not specified if parsed_schedule["kind"] == "once" and repeat is None: @@ -2526,6 +2537,25 @@ def update_job(job_id: str, updates: Dict[str, Any]) -> Optional[Dict[str, Any]] updates["reasoning_effort"] ) + # Normalize repeat the same way create_job does. Callers pass + # either the stored dict shape ({"times": N, "completed": M}) or + # a bare value ("forever", "once", 3, "3"); bare values coerce + # through normalize_repeat_value and preserve the completed + # counter. A raw string stored here previously broke + # mark_job_run ('str' has no .get) and repeat accounting. + if "repeat" in updates: + _rp = updates["repeat"] + if isinstance(_rp, dict): + _rp = dict(_rp) + _rp["times"] = normalize_repeat_value(_rp.get("times")) + _rp.setdefault("completed", (job.get("repeat") or {}).get("completed", 0)) + updates["repeat"] = _rp + else: + updates["repeat"] = { + "times": normalize_repeat_value(_rp), + "completed": (job.get("repeat") or {}).get("completed", 0), + } + previous_inference_axes = _normalized_inference_axes(job) updated = _apply_skill_fields({**job, **updates}) diff --git a/tests/cron/test_jobs.py b/tests/cron/test_jobs.py index 4218c52cbc..6a3492f390 100644 --- a/tests/cron/test_jobs.py +++ b/tests/cron/test_jobs.py @@ -412,6 +412,32 @@ class TestJobCRUD: with pytest.raises(ValueError, match="Invalid repeat"): create_job(prompt="Bad", schedule="every 1h", repeat="banana") + def test_update_repeat_string_forms_coerced(self, tmp_cron_dir): + """The UPDATE path must coerce repeat the same way create does. + + Before this fix, update_job({"repeat": "forever"}) stored the raw + string, and the next mark_job_run died with + "'str' object has no attribute 'get'". Same class as the create-path + TypeError (#66824/#64520/#7142/#71987/#95706) — the bare-value and + dict shapes both route through normalize_repeat_value now. + """ + from cron.jobs import mark_job_run, update_job + + job = create_job(prompt="t", schedule="every 1h", repeat=2) + mark_job_run(job["id"], success=True) # completed=1 + + updated = update_job(job["id"], {"repeat": "forever"}) + assert updated["repeat"]["times"] is None + assert updated["repeat"]["completed"] == 1 # counter preserved + mark_job_run(job["id"], success=True) # must not raise + + updated = update_job(job["id"], {"repeat": {"times": "3"}}) + assert updated["repeat"]["times"] == 3 + assert updated["repeat"]["completed"] == 2 + + with pytest.raises(ValueError, match="Invalid repeat"): + update_job(job["id"], {"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) diff --git a/tools/cronjob_tools.py b/tools/cronjob_tools.py index 5994220d62..bc9df41bbf 100644 --- a/tools/cronjob_tools.py +++ b/tools/cronjob_tools.py @@ -1917,8 +1917,12 @@ def cronjob( ) updates["no_agent"] = target_no_agent if repeat is not None: - # Normalize: treat 0 or negative as None (infinite) - normalized_repeat = None if repeat <= 0 else repeat + # Coerce string forms ('forever'/'once'/'3') and 0/negative + # via the shared chokepoint — a bare `repeat <= 0` here + # raised TypeError for string repeats on the UPDATE path + # (create was fixed first; same class). + from cron.jobs import normalize_repeat_value + normalized_repeat = normalize_repeat_value(repeat) repeat_state = dict(job.get("repeat") or {}) repeat_state["times"] = normalized_repeat updates["repeat"] = repeat_state