Skip to content

fix(cron): accept repeat=forever on cronjob create - #85594

Open
686f6c61 wants to merge 1 commit into
NousResearch:mainfrom
686f6c61:fix/85383-cronjob-repeat-forever
Open

fix(cron): accept repeat=forever on cronjob create#85594
686f6c61 wants to merge 1 commit into
NousResearch:mainfrom
686f6c61:fix/85383-cronjob-repeat-forever

Conversation

@686f6c61

@686f6c61 686f6c61 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

What breaks

cronjob(action='create', …, repeat='forever') raises:

TypeError: '<=' not supported between instances of 'str' and 'int'

Omitting repeat succeeds and the tool then returns "repeat": "forever". repeat=1 also works. So the documented sentinel is what the API itself prints, but it cannot be passed back in.

Why

create_job() and the update path did:

if repeat is not None and repeat <= 0:
    repeat = None

None is infinite; <= 0 was only meant for numeric “run forever”. The JSON schema advertised repeat as integer, but models (and the tool’s own success payload) use the string "forever". The comparison never ran a type check first.

What this changes

cron.jobs.normalize_repeat_count() accepts:

  • None / "forever" / "infinite" / "inf" / empty → None (infinite)
  • 0 or negative → None
  • a positive int (or numeric string) → that count

Create and update both go through it. The schema is now integer | string so the documented value is valid input.

Integer repeat=1 is unchanged ("once" in the formatted job).

Verify

pytest tests/tools/test_cronjob_tools.py::TestUnifiedCronjobTool::test_create_accepts_repeat_forever_string \
       tests/tools/test_cronjob_tools.py::TestUnifiedCronjobTool::test_create_repeat_numeric_still_works
# 2 passed

Not runtime-tested through a live gateway cron tick — the failure was at create time, before the scheduler.

Fixes #85383

The tool schema and success payload use the string "forever" for
infinite jobs, but create/update compared the raw value with <= 0.
Passing the documented sentinel raised TypeError.

Fixes NousResearch#85383
@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 13, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #7216: both normalize string repeat values before the create/update numeric comparisons.

@686f6c61

Copy link
Copy Markdown
Contributor Author

Same failure class as open #7216 (normalize_repeat_value before <= 0; also treats non-numeric strings as unset). That branch is dirty against current main (last push 2026-07-12).

This tip is the same coerce on today’s create_job / update path, plus the schema allowing string so "forever" is valid input (the value the tool already returns). int("forever") in #7216 also lands on None; we name that sentinel instead of relying on ValueError.

Happy to close this if maintainers rebase #7216 — diagnosis credit stays there.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(cron): accept repeat=forever on cronjob create

  1. cron/jobs.py normalize_repeat_countint(repeat) truncates floats silently: repeat=2.72. The schema is now integer|string, but the tool receives JSON-parsed args and a model can still send 2.5, which parses to a float. Consider rejecting non-integral floats (float.is_integer()), and note int(float('inf')) raises OverflowError (Python's json.loads accepts Infinity), which would surface as a generic tool error instead of the intended message.

  2. The string aliases {"", "forever", "infinite", "inf", "none", "null"} are more generous than the documented "forever" — in particular "none"/"null" mapping to infinite is counterintuitive (a caller writing "none" likely means "no repeat", which here means forever). Consider trimming the alias set to the documented values, or reversing the meaning of "none"/"null" to "once".

  3. tools/cronjob_tools.pynormalize_repeat_count is invoked in both create_job and the update path (idempotent, good). The registry dispatch wraps any ValueError into a clean tool error, so bad input fails gracefully; for consistency with the rest of the tool's validation style, returning tool_error(...) instead of raising would keep the message in the same shape as other validation failures.

@Enough1122

Copy link
Copy Markdown
Contributor

@C:/Users/admin/AppData/Local/Temp/opencode/review-85594.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cronjob create fails when repeat='forever' is passed explicitly

3 participants