diff --git a/nanobot/agent/tools/cron.py b/nanobot/agent/tools/cron.py index f1af59d7..111bb1c8 100644 --- a/nanobot/agent/tools/cron.py +++ b/nanobot/agent/tools/cron.py @@ -5,41 +5,71 @@ from datetime import datetime from typing import Any from nanobot.agent.tools.base import Tool, tool_parameters -from nanobot.agent.tools.schema import BooleanSchema, IntegerSchema, StringSchema, tool_parameters_schema +from nanobot.agent.tools.schema import ( + BooleanSchema, + IntegerSchema, + StringSchema, + tool_parameters_schema, +) from nanobot.cron.service import CronService from nanobot.cron.types import CronJob, CronJobState, CronSchedule - -@tool_parameters( - tool_parameters_schema( - action=StringSchema("Action to perform", enum=["add", "list", "remove"]), - name=StringSchema( - "Optional short human-readable label for the job " - "(e.g., 'weather-monitor', 'daily-standup'). Defaults to first 30 chars of message." - ), - message=StringSchema( - "REQUIRED when action='add'. Instruction for the agent to execute when the job triggers " - "(e.g., 'Send a reminder to WeChat: xxx' or 'Check system status and report'). " - "Not used for action='list' or action='remove'." - ), - every_seconds=IntegerSchema(0, description="Interval in seconds (for recurring tasks)"), - cron_expr=StringSchema("Cron expression like '0 9 * * *' (for scheduled tasks)"), - tz=StringSchema( - "Optional IANA timezone for cron expressions (e.g. 'America/Vancouver'). " - "When omitted with cron_expr, the tool's default timezone applies." - ), - at=StringSchema( - "ISO datetime for one-time execution (e.g. '2026-02-12T10:30:00'). " - "Naive values use the tool's default timezone." - ), - deliver=BooleanSchema( - description="Whether to deliver the execution result to the user channel (default true)", - default=True, - ), - job_id=StringSchema("REQUIRED when action='remove'. Job ID to remove (obtain via action='list')."), - required=["action"], - ) +_CRON_PARAMETERS = tool_parameters_schema( + action=StringSchema("Action to perform", enum=["add", "list", "remove"]), + name=StringSchema( + "Optional short human-readable label for the job " + "(e.g., 'weather-monitor', 'daily-standup'). Defaults to first 30 chars of message." + ), + message=StringSchema( + "REQUIRED when action='add'. Instruction for the agent to execute when the job triggers " + "(e.g., 'Send a reminder to WeChat: xxx' or 'Check system status and report'). " + "Not used for action='list' or action='remove'." + ), + every_seconds=IntegerSchema(0, description="Interval in seconds (for recurring tasks)"), + cron_expr=StringSchema("Cron expression like '0 9 * * *' (for scheduled tasks)"), + tz=StringSchema( + "Optional IANA timezone for cron expressions (e.g. 'America/Vancouver'). " + "When omitted with cron_expr, the tool's default timezone applies." + ), + at=StringSchema( + "ISO datetime for one-time execution (e.g. '2026-02-12T10:30:00'). " + "Naive values use the tool's default timezone." + ), + deliver=BooleanSchema( + description="Whether to deliver the execution result to the user channel (default true)", + default=True, + ), + job_id=StringSchema("REQUIRED when action='remove'. Job ID to remove (obtain via action='list')."), + required=["action"], + description=( + "Action-specific parameters: add requires a non-empty message plus one schedule " + "(every_seconds, cron_expr, or at); remove requires job_id; list only needs action." + ), ) +_CRON_PARAMETERS["oneOf"] = [ + { + "properties": { + "action": {"enum": ["add"]}, + "message": {"type": "string", "minLength": 1}, + }, + "required": ["action", "message"], + }, + { + "properties": { + "action": {"enum": ["list"]}, + }, + "required": ["action"], + }, + { + "properties": { + "action": {"enum": ["remove"]}, + }, + "required": ["action", "job_id"], + }, +] + + +@tool_parameters(_CRON_PARAMETERS) class CronTool(Tool): """Tool to schedule reminders and recurring tasks.""" @@ -95,6 +125,15 @@ class CronTool(Tool): f"If tz is omitted, cron expressions and naive ISO times default to {self._default_timezone}." ) + def validate_params(self, params: dict[str, Any]) -> list[str]: + errors = super().validate_params(params) + action = params.get("action") + if action == "add" and not str(params.get("message") or "").strip(): + errors.append("message is required when action='add'") + if action == "remove" and not str(params.get("job_id") or "").strip(): + errors.append("job_id is required when action='remove'") + return errors + async def execute( self, action: str, @@ -130,8 +169,8 @@ class CronTool(Tool): ) -> str: if not message: return ( - "Error: cron action='add' requires a non-empty 'message' " - "parameter describing what to do when the job triggers " + "Error: cron action='add' requires a non-empty 'message' parameter " + "describing what to do when the job triggers " "(e.g. the reminder text). Retry including message=\"...\"." ) if not self._channel or not self._chat_id: diff --git a/tests/cron/test_cron_tool_list.py b/tests/cron/test_cron_tool_list.py index 86f3055c..a3ee9b1a 100644 --- a/tests/cron/test_cron_tool_list.py +++ b/tests/cron/test_cron_tool_list.py @@ -7,7 +7,6 @@ import pytest from nanobot.agent.tools.cron import CronTool from nanobot.cron.service import CronService from nanobot.cron.types import CronJob, CronJobState, CronPayload, CronSchedule -from tests.test_openai_api import pytest_plugins def _make_tool(tmp_path) -> CronTool: @@ -346,6 +345,47 @@ def test_add_job_can_disable_delivery(tmp_path) -> None: assert job.payload.deliver is False +def test_cron_schema_advertises_action_specific_requirements(tmp_path) -> None: + tool = _make_tool(tmp_path) + + assert tool.parameters["required"] == ["action"] + assert tool.parameters["oneOf"] == [ + { + "properties": { + "action": {"enum": ["add"]}, + "message": {"type": "string", "minLength": 1}, + }, + "required": ["action", "message"], + }, + { + "properties": {"action": {"enum": ["list"]}}, + "required": ["action"], + }, + { + "properties": {"action": {"enum": ["remove"]}}, + "required": ["action", "job_id"], + }, + ] + + +def test_validate_params_requires_message_only_for_add(tmp_path) -> None: + tool = _make_tool(tmp_path) + + assert "message is required when action='add'" in tool.validate_params({"action": "add"}) + assert tool.validate_params({"action": "list"}) == [] + assert "job_id is required when action='remove'" in tool.validate_params({"action": "remove"}) + + +def test_add_job_empty_message_returns_actionable_error(tmp_path) -> None: + tool = _make_tool(tmp_path) + tool.set_context("telegram", "chat-1") + + result = tool._add_job(None, "", 60, None, None, None) + + assert "action='add' requires a non-empty 'message'" in result + assert "Retry including message=" in result + + def test_list_excludes_disabled_jobs(tmp_path) -> None: tool = _make_tool(tmp_path) job = tool._cron.add_job(