fix(cronjob): accept schedule param aliases (cron, when, ...) (#34120) - #53246
fix(cronjob): accept schedule param aliases (cron, when, ...) (#34120)#53246teknium1 wants to merge 2 commits into
Conversation
Fixes #34120. The 'cronjob' tool consistently rejected create calls with 'schedule is required for create' even when the user/agent included a schedule parameter — because the model (notably grok-4.3 on Discord) emitted the parameter under a non-canonical name like 'cron', 'cron_expression', or 'when' that the handler's args.get('schedule') lookup didn't see. Fix: add a _coalesce_arg helper that maps common synonym keys to the canonical kwargs at the registry handler boundary. Synonyms accepted: schedule ← cron, cron_expression, cron_schedule, when, time, frequency, interval prompt ← instruction, task, message job_id ← id, jobId, cron_id deliver ← delivery, deliver_to, destination, target skills ← skill_list, skill_names enabled_toolsets ← toolsets, tools no_agent ← script_only, no_llm Canonical always wins when present (and non-empty/None) so this is non-breaking. Empty-string or null canonical falls through to alias lookup, covering the grok-4.3 case where the model emits BOTH schedule='' AND cron='0 9 * * *' in the same tool call. Also improves the missing-schedule error message to list accepted aliases inline so user/agent can self-debug without grepping source. Adds 11 alias-coalesce tests + 1 error-message test in two new classes: TestCoalesceArgAliases (10) and TestCronjobMissingScheduleErrorMessage (1). All 68 tests in test_cronjob_tools.py pass. Co-authored-by: Cursor <cursoragent@cursor.com>
Follow-up to the #34120 alias-coalescing salvage. "message" is too generic a key to safely map onto the cron prompt — a model can emit it meaning something unrelated. The remaining "instruction"/"task" aliases are unambiguous. Canonical "prompt" still always wins.
🔎 Lint report:
|
alt-glitch
added
type/bug
|
Thanks for rebasing this onto main and keeping the attribution — appreciated. The alias-normalization approach here matches the intent of #34255; happy to see it land against current main. LGTM from my side. |
|
Closing — we don't add alias/synonym coverage to work around models emitting tool-call arguments under the wrong key. The schema declares the parameter as Appreciate the work, @Bartok9 — this just isn't a direction we take. |
|
Completely fair call, and thanks for taking the time to explain the reasoning rather than just closing it — that's appreciated. You're right: a model emitting the value under the wrong key is a schema-compliance failure on the model's side, and a coalescing layer that accepts every synonym would be open-ended and would mask the real problem instead of surfacing it. Failing loudly with "schedule is required" is the correct behavior. I'd rather the schema stay strict than grow an ever-expanding compatibility shim. Appreciate the clear philosophy — it'll sharpen what I bring next. Thanks again 🙏 |
3 participants
Summary
cronjob(action=create)now accepts the schedule under common synonym keys some models emit (cron,when,cron_expression, …) instead of failing with "schedule is required for create" — fixes #34120.Root cause: the handler lambda read every argument by its exact canonical key (
args.get("schedule")). Models that emitted the cron expression undercron/when, or setschedule=""alongsidecron="0 9 * * *", hit the missing-schedule rejection even though a valid schedule was present in the same tool call.Changes
tools/cronjob_tools.py: add_coalesce_arg()+_CRONJOB_ARG_ALIASES; rewire the registry handler lambda to coalesce aliases forschedule,prompt,job_id,deliver,skills,enabled_toolsets,no_agent. Canonical key always wins; empty/None falls through to aliases. Missing-schedule error now lists accepted aliases + examples."message"from thepromptalias set — too generic a key to safely map onto the cron prompt.instruction/taskretained.tests/tools/test_cronjob_tools.py: coalesce-precedence, per-alias, empty-string fall-through, scoping, and error-message tests.Validation
schedule="..."cron="..."schedule="" , cron="..."when="every 2h"message→ promptLive before/after verified through the real
registry.dispatchpath against a tempHERMES_HOME(no mocks). Targeted suite: 79/79 pass.Salvaged from #34255 by @Bartok9 — his authorship preserved via cherry-pick. Supersedes #49792 (which made
schedule/job_idglobally required in the schema — incorrect, since those are action-specific and would break everylist/remove/pause/resume/runcall).Infographic