diff --git a/tests/tools/test_cronjob_tools.py b/tests/tools/test_cronjob_tools.py index 08c82f375134f..b3091eb85a692 100644 --- a/tests/tools/test_cronjob_tools.py +++ b/tests/tools/test_cronjob_tools.py @@ -581,3 +581,91 @@ def test_gateway_origin_no_notice(self, monkeypatch): ) assert created["deliver"] == "origin" assert "local-only cron job" not in created["message"] + + +# ========================================================================= +# #34120: Tool-call argument aliases & better missing-schedule error +# ========================================================================= + +class TestCoalesceArgAliases: + """#34120: The cronjob tool handler must accept common parameter aliases + that some models (notably grok-4.3 and several xAI variants) emit when + they don't perfectly match the JSON schema's field names. Without this, + a user running 'create a cron that posts daily at 7:30 PM' would see + the tool call rejected with 'schedule is required for create' even + though the model included 'cron' or 'when' in the same call. + """ + + def test_canonical_schedule_takes_precedence(self): + from tools.cronjob_tools import _coalesce_arg + args = {"schedule": "0 9 * * *", "cron": "should-not-win"} + assert _coalesce_arg(args, "schedule") == "0 9 * * *" + + def test_cron_alias_used_when_canonical_missing(self): + from tools.cronjob_tools import _coalesce_arg + args = {"cron": "0 9 * * *"} + assert _coalesce_arg(args, "schedule") == "0 9 * * *" + + def test_cron_expression_alias(self): + from tools.cronjob_tools import _coalesce_arg + args = {"cron_expression": "30 19 * * *"} + assert _coalesce_arg(args, "schedule") == "30 19 * * *" + + def test_when_alias(self): + from tools.cronjob_tools import _coalesce_arg + args = {"when": "every 2h"} + assert _coalesce_arg(args, "schedule") == "every 2h" + + def test_empty_canonical_falls_through_to_alias(self): + """The grok-4.3 #34120 case: model emits BOTH schedule='' AND + cron='0 9 * * *'. The canonical empty string must not block the + alias lookup.""" + from tools.cronjob_tools import _coalesce_arg + args = {"schedule": "", "cron": "0 9 * * *"} + assert _coalesce_arg(args, "schedule") == "0 9 * * *" + + def test_null_canonical_falls_through_to_alias(self): + from tools.cronjob_tools import _coalesce_arg + args = {"schedule": None, "cron": "0 9 * * *"} + assert _coalesce_arg(args, "schedule") == "0 9 * * *" + + def test_prompt_alias_instruction(self): + from tools.cronjob_tools import _coalesce_arg + args = {"instruction": "Run the daily report"} + assert _coalesce_arg(args, "prompt") == "Run the daily report" + + def test_prompt_alias_task(self): + from tools.cronjob_tools import _coalesce_arg + args = {"task": "Run the daily report"} + assert _coalesce_arg(args, "prompt") == "Run the daily report" + + def test_deliver_alias(self): + from tools.cronjob_tools import _coalesce_arg + args = {"destination": "discord:123:456"} + assert _coalesce_arg(args, "deliver") == "discord:123:456" + + def test_no_alias_returns_none(self): + from tools.cronjob_tools import _coalesce_arg + args = {"unrelated_key": "value"} + assert _coalesce_arg(args, "schedule") is None + + def test_no_match_for_other_canonical(self): + """Aliases must be scoped — a 'cron' key should not satisfy a + 'prompt' coalesce.""" + from tools.cronjob_tools import _coalesce_arg + args = {"cron": "0 9 * * *"} + assert _coalesce_arg(args, "prompt") is None + + +class TestCronjobMissingScheduleErrorMessage: + """The error message must list accepted aliases so a user / agent can + self-debug without grepping source.""" + + def test_error_lists_accepted_aliases(self): + result = cronjob(action="create", prompt="Do the thing") + # tool_error returns a string with the failure shape; assert the + # alias hint is included. + assert "schedule is required for create" in result + assert "cron" in result + assert "when" in result + assert "0 9 * * *" in result # example included diff --git a/tools/cronjob_tools.py b/tools/cronjob_tools.py index 999297c20bb58..7987709b3c85e 100644 --- a/tools/cronjob_tools.py +++ b/tools/cronjob_tools.py @@ -597,7 +597,20 @@ def cronjob( if normalized == "create": if not schedule: - return tool_error("schedule is required for create", success=False) + # When the schedule is missing we previously surfaced only + # "schedule is required for create" — unhelpful when the + # model thought it had passed one under an alias name (cron, + # when, etc., see #34120). The handler's lambda layer now + # coalesces those aliases, but if both canonical and alias + # were missing OR empty we land here. Include a brief hint. + return tool_error( + "schedule is required for create. Pass it as 'schedule' " + "(canonical), or one of the accepted aliases: " + "cron, cron_expression, cron_schedule, when, time, " + "frequency, interval. Examples: '30m', 'every 2h', " + "'0 9 * * *', '2026-06-01T09:00:00'.", + success=False, + ) canonical_skills = _canonical_skills(skill, skills) _no_agent = bool(no_agent) # Job-shape validation differs by mode: @@ -1001,30 +1014,66 @@ def check_cronjob_requirements() -> bool: # --- Registry --- from tools.registry import registry, tool_error +# Tool-call argument aliases. Some models (notably grok-4.3 and several xAI +# variants observed in #34120) emit the cron schedule under non-canonical +# keys like ``cron``, ``cron_expression``, or ``when`` even when the JSON +# schema clearly names the field ``schedule``. Rather than fight every +# model's naming preference, accept the common synonyms and normalize them +# to the canonical kwargs. This is harmless when the canonical key is also +# present (we prefer canonical over alias). +_CRONJOB_ARG_ALIASES: dict[str, tuple[str, ...]] = { + "schedule": ("cron", "cron_expression", "cron_schedule", "when", "time", "frequency", "interval"), + "prompt": ("instruction", "task"), + "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"), +} + + +def _coalesce_arg(args: dict, canonical: str) -> Any: + """Return ``args[canonical]`` if present, else the first matching alias. + + Empty strings and ``None`` count as "not present" so an explicit + ``schedule=""`` from the model still falls through to aliases (which + addresses the #34120 case where the model emits ``schedule: ""`` AND + ``cron: "0 9 * * *"`` in the same tool call). + """ + val = args.get(canonical) + if val not in (None, ""): + return val + for alias in _CRONJOB_ARG_ALIASES.get(canonical, ()): + alias_val = args.get(alias) + if alias_val not in (None, ""): + return alias_val + return val # preserve original None/"" so downstream falsy check fires correctly + + registry.register( name="cronjob", toolset="cronjob", schema=CRONJOB_SCHEMA, handler=lambda args, **kw: (lambda _mo=_resolve_model_override(args.get("model")): cronjob( action=args.get("action", ""), - job_id=args.get("job_id"), - prompt=args.get("prompt"), - schedule=args.get("schedule"), + job_id=_coalesce_arg(args, "job_id"), + prompt=_coalesce_arg(args, "prompt"), + schedule=_coalesce_arg(args, "schedule"), name=args.get("name"), repeat=args.get("repeat"), - deliver=args.get("deliver"), + deliver=_coalesce_arg(args, "deliver"), include_disabled=args.get("include_disabled", True), skill=args.get("skill"), - skills=args.get("skills"), + skills=_coalesce_arg(args, "skills"), model=_mo[1], provider=_mo[0] or args.get("provider"), base_url=args.get("base_url"), reason=args.get("reason"), script=args.get("script"), context_from=args.get("context_from"), - enabled_toolsets=args.get("enabled_toolsets"), + enabled_toolsets=_coalesce_arg(args, "enabled_toolsets"), workdir=args.get("workdir"), - no_agent=args.get("no_agent"), + no_agent=_coalesce_arg(args, "no_agent"), task_id=kw.get("task_id"), ))(), check_fn=check_cronjob_requirements,