diff --git a/skills/ironclaw-workflow-orchestrator/references/workflow-routines.md b/skills/ironclaw-workflow-orchestrator/references/workflow-routines.md index 8afa857d7..5e64a2b2c 100644 --- a/skills/ironclaw-workflow-orchestrator/references/workflow-routines.md +++ b/skills/ironclaw-workflow-orchestrator/references/workflow-routines.md @@ -8,15 +8,21 @@ Replace `{{...}}` placeholders before use. { "name": "wf-issue-plan", "description": "Create implementation plan when a new issue arrives", - "trigger_type": "system_event", - "event_source": "github", - "event_type": "issue.opened", - "event_filters": { - "repository_name": "{{repository}}" - }, - "action_type": "full_job", "prompt": "For issue #{{issue_number}} in {{repository}}, produce a concrete implementation plan with milestones, edge cases, and tests. Post/update an issue comment with the plan.", - "cooldown_secs": 30 + "request": { + "kind": "system_event", + "source": "github", + "event_type": "issue.opened", + "filters": { + "repository_name": "{{repository}}" + } + }, + "execution": { + "mode": "full_job" + }, + "advanced": { + "cooldown_secs": 30 + } } ``` @@ -28,16 +34,22 @@ Trigger per-maintainer by creating one routine per handle, or maintain a shared { "name": "wf-maintainer-comment-gate-{{maintainer}}", "description": "React to maintainer guidance comments on issues/PRs", - "trigger_type": "system_event", - "event_source": "github", - "event_type": "pr.comment.created", - "event_filters": { - "repository_name": "{{repository}}", - "comment_author": "{{maintainer}}" - }, - "action_type": "full_job", "prompt": "Read the maintainer comment and decide: update plan or start/continue implementation. If plan changes are requested, edit the plan artifact first. If implementation is requested, continue on the feature branch and update PR status/comment.", - "cooldown_secs": 20 + "request": { + "kind": "system_event", + "source": "github", + "event_type": "pr.comment.created", + "filters": { + "repository_name": "{{repository}}", + "comment_author": "{{maintainer}}" + } + }, + "execution": { + "mode": "full_job" + }, + "advanced": { + "cooldown_secs": 20 + } } ``` @@ -47,15 +59,21 @@ Trigger per-maintainer by creating one routine per handle, or maintain a shared { "name": "wf-pr-monitor-loop", "description": "Keep PR healthy: address review comments and refresh branch", - "trigger_type": "system_event", - "event_source": "github", - "event_type": "pr.synchronize", - "event_filters": { - "repository_name": "{{repository}}" - }, - "action_type": "full_job", "prompt": "For PR #{{pr_number}}, collect open review comments and unresolved threads, apply fixes, push branch updates, and summarize remaining blockers. If conflict with {{main_branch}}, rebase/merge from origin/{{main_branch}} and resolve safely.", - "cooldown_secs": 20 + "request": { + "kind": "system_event", + "source": "github", + "event_type": "pr.synchronize", + "filters": { + "repository_name": "{{repository}}" + } + }, + "execution": { + "mode": "full_job" + }, + "advanced": { + "cooldown_secs": 20 + } } ``` @@ -65,16 +83,22 @@ Trigger per-maintainer by creating one routine per handle, or maintain a shared { "name": "wf-ci-fix-loop", "description": "Fix failing CI checks on active PRs", - "trigger_type": "system_event", - "event_source": "github", - "event_type": "ci.check_run.completed", - "event_filters": { - "repository_name": "{{repository}}", - "ci_conclusion": "failure" - }, - "action_type": "full_job", "prompt": "Find failing check details for PR #{{pr_number}}, implement minimal safe fixes, rerun or await CI, and post concise status updates. Prioritize deterministic and test-backed fixes.", - "cooldown_secs": 20 + "request": { + "kind": "system_event", + "source": "github", + "event_type": "ci.check_run.completed", + "filters": { + "repository_name": "{{repository}}", + "ci_conclusion": "failure" + } + }, + "execution": { + "mode": "full_job" + }, + "advanced": { + "cooldown_secs": 20 + } } ``` @@ -84,11 +108,17 @@ Trigger per-maintainer by creating one routine per handle, or maintain a shared { "name": "wf-staging-batch-review", "description": "Batch correctness review through staging, then merge to main", - "trigger_type": "cron", - "schedule": "0 0 */{{batch_interval_hours}} * * *", - "action_type": "full_job", "prompt": "Every cycle: list ready PRs, merge ready ones into {{staging_branch}}, run deep correctness analysis in batch, fix discovered issues on affected branches, ensure CI green, then merge {{staging_branch}} into {{main_branch}} if clean.", - "cooldown_secs": 120 + "request": { + "kind": "cron", + "schedule": "0 0 */{{batch_interval_hours}} * * *" + }, + "execution": { + "mode": "full_job" + }, + "advanced": { + "cooldown_secs": 120 + } } ``` @@ -98,16 +128,22 @@ Trigger per-maintainer by creating one routine per handle, or maintain a shared { "name": "wf-learning-memory", "description": "Capture merge learnings into shared memory", - "trigger_type": "system_event", - "event_source": "github", - "event_type": "pr.closed", - "event_filters": { - "repository_name": "{{repository}}", - "pr_merged": "true" - }, - "action_type": "full_job", "prompt": "From merged PR #{{pr_number}}, extract preventable mistakes, reviewer themes, CI failure causes, and successful patterns. Write/update a shared memory doc with actionable rules to reduce cycle time and regressions.", - "cooldown_secs": 30 + "request": { + "kind": "system_event", + "source": "github", + "event_type": "pr.closed", + "filters": { + "repository_name": "{{repository}}", + "pr_merged": "true" + } + }, + "execution": { + "mode": "full_job" + }, + "advanced": { + "cooldown_secs": 30 + } } ``` @@ -115,7 +151,7 @@ Trigger per-maintainer by creating one routine per handle, or maintain a shared ```json { - "source": "github", + "event_source": "github", "event_type": "issue.opened", "payload": { "repository_name": "{{repository}}", diff --git a/src/tools/builtin/routine.rs b/src/tools/builtin/routine.rs index 347cb4ff0..1d57af77b 100644 --- a/src/tools/builtin/routine.rs +++ b/src/tools/builtin/routine.rs @@ -9,11 +9,13 @@ //! - `routine_history` - View past runs //! - `event_emit` - Emit a structured system event to `system_event`-triggered routines +use std::collections::HashMap; use std::sync::Arc; use std::time::Duration; use async_trait::async_trait; use chrono::Utc; +use serde_json::{Map, Value}; use uuid::Uuid; use crate::agent::routine::{ @@ -24,133 +26,746 @@ use crate::context::JobContext; use crate::db::Database; use crate::tools::tool::{ApprovalRequirement, Tool, ToolError, ToolOutput, require_str}; -pub(crate) fn routine_create_parameters_schema() -> serde_json::Value { +// ==================== routine_create ==================== + +#[derive(Debug, Clone, PartialEq, Eq)] +enum NormalizedTriggerRequest { + Cron { + schedule: String, + timezone: Option, + }, + Manual, + MessageEvent { + pattern: String, + channel: Option, + }, + SystemEvent { + source: String, + event_type: String, + filters: HashMap, + }, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum NormalizedExecutionMode { + Lightweight, + FullJob, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct NormalizedExecutionRequest { + mode: NormalizedExecutionMode, + context_paths: Vec, + use_tools: bool, + max_tool_rounds: u32, + tool_permissions: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct NormalizedDeliveryRequest { + channel: Option, + user: String, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct NormalizedRoutineCreateRequest { + name: String, + description: String, + prompt: String, + trigger: NormalizedTriggerRequest, + execution: NormalizedExecutionRequest, + delivery: NormalizedDeliveryRequest, + cooldown_secs: u64, +} + +fn routine_request_properties() -> Value { + serde_json::json!({ + "kind": { + "type": "string", + "enum": ["cron", "manual", "message_event", "system_event"], + "description": "How the routine should start." + }, + "schedule": { + "type": "string", + "description": "Cron expression for request.kind='cron'. Uses 6-field cron: second minute hour day month weekday." + }, + "timezone": { + "type": "string", + "description": "IANA timezone for request.kind='cron', such as 'America/New_York'." + }, + "pattern": { + "type": "string", + "description": "Regex pattern for request.kind='message_event'." + }, + "channel": { + "type": "string", + "description": "Optional channel filter for request.kind='message_event'." + }, + "source": { + "type": "string", + "description": "Event source namespace for request.kind='system_event', such as 'github'." + }, + "event_type": { + "type": "string", + "description": "Event type for request.kind='system_event', such as 'issue.opened'." + }, + "filters": { + "type": "object", + "properties": {}, + "additionalProperties": { + "type": ["string", "number", "boolean"] + }, + "description": "Optional exact-match filters for request.kind='system_event'. Only top-level string, number, and boolean payload fields are matched." + } + }) +} + +fn execution_properties() -> Value { serde_json::json!({ + "mode": { + "type": "string", + "enum": ["lightweight", "full_job"], + "description": "Execution mode. 'lightweight' is the default. 'full_job' runs a multi-turn autonomous job." + }, + "context_paths": { + "type": "array", + "items": { "type": "string" }, + "description": "Workspace paths to preload for lightweight routines." + }, + "use_tools": { + "type": "boolean", + "description": "Only applies to lightweight mode. When true, safe non-approval tools are available." + }, + "max_tool_rounds": { + "type": "integer", + "minimum": 1, + "maximum": crate::agent::routine::MAX_TOOL_ROUNDS_LIMIT, + "default": 3, + "description": "Only applies when execution.mode='lightweight' and use_tools=true. Runtime-capped to prevent loops." + }, + "tool_permissions": { + "type": "array", + "items": { "type": "string" }, + "description": "Only applies when execution.mode='full_job'. These tools are pre-authorized for Always-approval checks." + } + }) +} + +fn delivery_properties() -> Value { + serde_json::json!({ + "channel": { + "type": "string", + "description": "Default channel for notifications and routine job message calls." + }, + "user": { + "type": "string", + "description": "Default user or target for notifications and routine job message calls. Defaults to 'default'." + } + }) +} + +fn advanced_properties() -> Value { + serde_json::json!({ + "cooldown_secs": { + "type": "integer", + "description": "Minimum seconds between automatic fires. Manual fires still bypass cooldown." + } + }) +} + +fn routine_create_schema(include_compatibility_aliases: bool) -> Value { + let mut schema = serde_json::json!({ "type": "object", "properties": { "name": { "type": "string", - "description": "Unique routine name, for example 'daily-pr-review'." - }, - "description": { - "type": "string", - "description": "Short summary of what the routine is for." - }, - "trigger_type": { - "type": "string", - "enum": ["cron", "event", "system_event", "manual"], - "description": "When the routine fires: 'cron' for schedules, 'event' for incoming messages, 'system_event' for structured emitted events, or 'manual' for explicit runs." - }, - "schedule": { - "type": "string", - "description": "Cron schedule for 'cron' triggers. Uses 6 fields: second minute hour day month weekday." - }, - "event_pattern": { - "type": "string", - "description": "Regex matched against incoming message text for 'event' triggers, for example '^bug\\\\b'." - }, - "event_channel": { - "type": "string", - "description": "Optional platform filter for 'event' triggers, for example 'telegram'. Omit to match any channel. Not a chat or thread ID." - }, - "event_source": { - "type": "string", - "description": "Structured event source for 'system_event' triggers, for example 'github'." - }, - "event_type": { - "type": "string", - "description": "Structured event type for 'system_event' triggers, for example 'issue.opened'." - }, - "event_filters": { - "type": "object", - "properties": {}, - "additionalProperties": { - "type": ["string", "number", "boolean"] - }, - "description": "Optional exact-match payload filters for 'system_event' triggers. Values can be strings, numbers, or booleans." + "description": "Unique name for the routine (e.g. 'daily-pr-review')." }, "prompt": { "type": "string", - "description": "Instructions for what the routine should do after it fires." - }, - "context_paths": { - "type": "array", - "items": { "type": "string" }, - "description": "Workspace paths to load as extra context before running the routine." + "description": "Instructions for what the routine should do when it fires." }, - "action_type": { + "description": { "type": "string", - "enum": ["lightweight", "full_job"], - "description": "Execution mode: 'lightweight' for one LLM turn or 'full_job' for a multi-step job with tools." - }, - "use_tools": { - "type": "boolean", - "description": "Enable safe tool use in 'lightweight' mode. Ignored for 'full_job'." + "description": "Optional human-readable summary of what the routine does." }, - "max_tool_rounds": { - "type": "integer", - "description": "Maximum tool-call rounds in 'lightweight' mode when 'use_tools' is true." - }, - "cooldown_secs": { - "type": "integer", - "description": "Minimum seconds between fires." - }, - "tool_permissions": { - "type": "array", - "items": { "type": "string" }, - "description": "Pre-authorized tool names for 'full_job' routines." + "request": { + "type": "object", + "description": "Canonical trigger config. Set request.kind first, then only fill fields that match that kind.", + "properties": routine_request_properties(), + "required": ["kind"] }, - "notify_channel": { - "type": "string", - "description": "Where routine output should be sent, for example 'telegram' or 'slack'. This does not control what triggers the routine." + "execution": { + "type": "object", + "description": "Optional execution settings. Omit for the default lightweight mode.", + "properties": execution_properties() }, - "notify_user": { - "type": "string", - "description": "Optional explicit user or destination to notify, for example a username or chat ID. Omit it to use the configured owner's last-seen target for that channel." + "delivery": { + "type": "object", + "description": "Optional delivery defaults for notifications and message tool calls inside routine jobs.", + "properties": delivery_properties() }, - "timezone": { - "type": "string", - "description": "IANA timezone used to evaluate 'cron' schedules, for example 'America/New_York'." + "advanced": { + "type": "object", + "description": "Optional advanced knobs. Most routines can omit this block.", + "properties": advanced_properties() } }, - "required": ["name", "trigger_type", "prompt"] - }) + "required": ["name", "prompt"] + }); + + if include_compatibility_aliases { + if let Some(properties) = schema.get_mut("properties").and_then(Value::as_object_mut) { + properties.insert( + "trigger_type".to_string(), + serde_json::json!({ + "type": "string", + "enum": ["cron", "event", "system_event", "manual"], + "description": "Compatibility alias for request.kind. Prefer request.kind." + }), + ); + properties.insert( + "schedule".to_string(), + serde_json::json!({ + "type": "string", + "description": "Compatibility alias for request.schedule. Prefer request.schedule." + }), + ); + properties.insert( + "timezone".to_string(), + serde_json::json!({ + "type": "string", + "description": "Compatibility alias for request.timezone. Prefer request.timezone." + }), + ); + properties.insert( + "event_pattern".to_string(), + serde_json::json!({ + "type": "string", + "description": "Compatibility alias for request.pattern when request.kind='message_event'." + }), + ); + properties.insert( + "event_channel".to_string(), + serde_json::json!({ + "type": "string", + "description": "Compatibility alias for request.channel when request.kind='message_event'." + }), + ); + properties.insert( + "event_source".to_string(), + serde_json::json!({ + "type": "string", + "description": "Compatibility alias for request.source when request.kind='system_event'." + }), + ); + properties.insert( + "event_type".to_string(), + serde_json::json!({ + "type": "string", + "description": "Compatibility alias for request.event_type when request.kind='system_event'." + }), + ); + properties.insert( + "event_filters".to_string(), + serde_json::json!({ + "type": "object", + "properties": {}, + "additionalProperties": { + "type": ["string", "number", "boolean"] + }, + "description": "Compatibility alias for request.filters when request.kind='system_event'." + }), + ); + properties.insert( + "action_type".to_string(), + serde_json::json!({ + "type": "string", + "enum": ["lightweight", "full_job"], + "description": "Compatibility alias for execution.mode." + }), + ); + properties.insert( + "context_paths".to_string(), + serde_json::json!({ + "type": "array", + "items": { "type": "string" }, + "description": "Compatibility alias for execution.context_paths." + }), + ); + properties.insert( + "use_tools".to_string(), + serde_json::json!({ + "type": "boolean", + "description": "Compatibility alias for execution.use_tools." + }), + ); + properties.insert( + "max_tool_rounds".to_string(), + serde_json::json!({ + "type": "integer", + "minimum": 1, + "maximum": crate::agent::routine::MAX_TOOL_ROUNDS_LIMIT, + "default": 3, + "description": "Compatibility alias for execution.max_tool_rounds." + }), + ); + properties.insert( + "tool_permissions".to_string(), + serde_json::json!({ + "type": "array", + "items": { "type": "string" }, + "description": "Compatibility alias for execution.tool_permissions." + }), + ); + properties.insert( + "notify_channel".to_string(), + serde_json::json!({ + "type": "string", + "description": "Compatibility alias for delivery.channel." + }), + ); + properties.insert( + "notify_user".to_string(), + serde_json::json!({ + "type": "string", + "description": "Compatibility alias for delivery.user." + }), + ); + properties.insert( + "cooldown_secs".to_string(), + serde_json::json!({ + "type": "integer", + "description": "Compatibility alias for advanced.cooldown_secs." + }), + ); + } + if let Some(schema_obj) = schema.as_object_mut() { + schema_obj.insert( + "anyOf".to_string(), + serde_json::json!([ + { "required": ["request"] }, + { "required": ["trigger_type"] } + ]), + ); + } + } else if let Some(required) = schema.get_mut("required").and_then(Value::as_array_mut) { + required.push(Value::String("request".to_string())); + } + + schema +} + +pub(crate) fn routine_create_parameters_schema() -> Value { + routine_create_schema(false) } -pub(crate) fn routine_update_parameters_schema() -> serde_json::Value { +fn routine_create_discovery_schema() -> Value { + routine_create_schema(true) +} + +pub(crate) fn routine_update_parameters_schema() -> Value { serde_json::json!({ "type": "object", "properties": { "name": { "type": "string", - "description": "Name of the routine to update." + "description": "Name of the routine to update" }, "enabled": { "type": "boolean", - "description": "Set to true to enable the routine or false to disable it." + "description": "Enable or disable the routine" }, "prompt": { "type": "string", - "description": "Replace the routine instructions for what it should do after it fires." + "description": "New prompt/instructions" }, "schedule": { "type": "string", - "description": "New cron schedule for existing 'cron' routines only. This does not convert other trigger types." + "description": "New cron schedule (for cron triggers)" }, "timezone": { "type": "string", - "description": "New IANA timezone for existing 'cron' routines only, for example 'America/New_York'." + "description": "IANA timezone for cron schedule (e.g. 'America/New_York'). Only valid for cron triggers." }, "description": { "type": "string", - "description": "Replace the routine summary." + "description": "New description" } }, "required": ["name"] }) } -// ==================== routine_create ==================== +fn nested_object<'a>(params: &'a Value, field: &str) -> Option<&'a Map> { + params.get(field).and_then(Value::as_object) +} + +fn string_field(params: &Value, group: &str, field: &str, aliases: &[&str]) -> Option { + nested_object(params, group) + .and_then(|obj| obj.get(field)) + .and_then(Value::as_str) + .map(String::from) + .or_else(|| { + aliases + .iter() + .find_map(|alias| params.get(*alias).and_then(Value::as_str).map(String::from)) + }) +} + +fn bool_field(params: &Value, group: &str, field: &str, aliases: &[&str]) -> Option { + nested_object(params, group) + .and_then(|obj| obj.get(field)) + .and_then(Value::as_bool) + .or_else(|| { + aliases + .iter() + .find_map(|alias| params.get(*alias).and_then(Value::as_bool)) + }) +} + +fn u64_field(params: &Value, group: &str, field: &str, aliases: &[&str]) -> Option { + nested_object(params, group) + .and_then(|obj| obj.get(field)) + .and_then(Value::as_u64) + .or_else(|| { + aliases + .iter() + .find_map(|alias| params.get(*alias).and_then(Value::as_u64)) + }) +} + +fn string_array_field(params: &Value, group: &str, field: &str, aliases: &[&str]) -> Vec { + nested_object(params, group) + .and_then(|obj| obj.get(field)) + .and_then(Value::as_array) + .or_else(|| { + aliases + .iter() + .find_map(|alias| params.get(*alias).and_then(Value::as_array)) + }) + .map(|arr| { + arr.iter() + .filter_map(|value| value.as_str().map(String::from)) + .collect() + }) + .unwrap_or_default() +} + +fn object_field( + params: &Value, + group: &str, + field: &str, + aliases: &[&str], +) -> Option> { + nested_object(params, group) + .and_then(|obj| obj.get(field)) + .and_then(Value::as_object) + .cloned() + .or_else(|| { + aliases + .iter() + .find_map(|alias| params.get(*alias).and_then(Value::as_object).cloned()) + }) +} + +fn validate_timezone_param(timezone: Option) -> Result, ToolError> { + timezone + .map(|tz| { + crate::timezone::parse_timezone(&tz) + .map(|_| tz.clone()) + .ok_or_else(|| { + ToolError::InvalidParameters(format!("invalid IANA timezone: '{tz}'")) + }) + }) + .transpose() +} + +fn parse_system_event_filters( + filters: Option>, +) -> Result, ToolError> { + let Some(obj) = filters else { + return Ok(HashMap::new()); + }; + + let mut parsed = HashMap::with_capacity(obj.len()); + for (key, value) in obj { + let rendered = crate::agent::routine::json_value_as_filter_string(&value).ok_or_else(|| { + ToolError::InvalidParameters(format!( + "system_event filters only support string, number, and boolean values (invalid '{key}')" + )) + })?; + parsed.insert(key, rendered); + } + + Ok(parsed) +} + +fn parse_routine_trigger(params: &Value) -> Result { + let kind = string_field(params, "request", "kind", &["trigger_type"]) + .map(|value| match value.as_str() { + "event" => "message_event".to_string(), + other => other.to_string(), + }) + .ok_or_else(|| { + ToolError::InvalidParameters( + "routine_create requires request.kind (canonical) or trigger_type (legacy)" + .to_string(), + ) + })?; + + match kind.as_str() { + "cron" => { + let schedule = + string_field(params, "request", "schedule", &["schedule"]).ok_or_else(|| { + ToolError::InvalidParameters("cron request requires 'schedule'".to_string()) + })?; + let timezone = validate_timezone_param(string_field( + params, + "request", + "timezone", + &["timezone"], + ))?; + next_cron_fire(&schedule, timezone.as_deref()) + .map_err(|e| ToolError::InvalidParameters(format!("invalid cron schedule: {e}")))?; + Ok(NormalizedTriggerRequest::Cron { schedule, timezone }) + } + "manual" => Ok(NormalizedTriggerRequest::Manual), + "message_event" => { + let pattern = string_field(params, "request", "pattern", &["event_pattern"]) + .ok_or_else(|| { + ToolError::InvalidParameters( + "message_event request requires 'pattern'".to_string(), + ) + })?; + regex::RegexBuilder::new(&pattern) + .size_limit(64 * 1024) + .build() + .map_err(|e| { + ToolError::InvalidParameters(format!("invalid or too complex regex: {e}")) + })?; + let channel = string_field(params, "request", "channel", &["event_channel"]); + Ok(NormalizedTriggerRequest::MessageEvent { pattern, channel }) + } + "system_event" => { + let source = + string_field(params, "request", "source", &["event_source"]).ok_or_else(|| { + ToolError::InvalidParameters( + "system_event request requires 'source'".to_string(), + ) + })?; + let event_type = string_field(params, "request", "event_type", &["event_type"]) + .ok_or_else(|| { + ToolError::InvalidParameters( + "system_event request requires 'event_type'".to_string(), + ) + })?; + let filters = parse_system_event_filters(object_field( + params, + "request", + "filters", + &["event_filters"], + ))?; + Ok(NormalizedTriggerRequest::SystemEvent { + source, + event_type, + filters, + }) + } + other => Err(ToolError::InvalidParameters(format!( + "unknown request.kind: {other}" + ))), + } +} + +fn parse_execution_mode(value: Option) -> Result { + match value.as_deref().unwrap_or("lightweight") { + "lightweight" => Ok(NormalizedExecutionMode::Lightweight), + "full_job" => Ok(NormalizedExecutionMode::FullJob), + other => Err(ToolError::InvalidParameters(format!( + "unknown execution mode: {other}" + ))), + } +} + +fn parse_routine_execution(params: &Value) -> Result { + let mode = parse_execution_mode(string_field(params, "execution", "mode", &["action_type"]))?; + let context_paths = + string_array_field(params, "execution", "context_paths", &["context_paths"]); + let use_tools = bool_field(params, "execution", "use_tools", &["use_tools"]).unwrap_or(false); + let max_tool_rounds = u64_field(params, "execution", "max_tool_rounds", &["max_tool_rounds"]) + .unwrap_or(3) + .clamp(1, crate::agent::routine::MAX_TOOL_ROUNDS_LIMIT as u64) + as u32; + let tool_permissions = string_array_field( + params, + "execution", + "tool_permissions", + &["tool_permissions"], + ); + + Ok(NormalizedExecutionRequest { + mode, + context_paths, + use_tools, + max_tool_rounds, + tool_permissions, + }) +} + +fn parse_routine_delivery(params: &Value) -> NormalizedDeliveryRequest { + NormalizedDeliveryRequest { + channel: string_field(params, "delivery", "channel", &["notify_channel"]), + user: string_field(params, "delivery", "user", &["notify_user"]) + .unwrap_or_else(|| "default".to_string()), + } +} + +fn parse_routine_create_request( + params: &Value, +) -> Result { + let name = require_str(params, "name")?.to_string(); + let prompt = require_str(params, "prompt")?.to_string(); + let description = params + .get("description") + .and_then(Value::as_str) + .unwrap_or("") + .to_string(); + let trigger = parse_routine_trigger(params)?; + let execution = parse_routine_execution(params)?; + let delivery = parse_routine_delivery(params); + let cooldown_secs = + u64_field(params, "advanced", "cooldown_secs", &["cooldown_secs"]).unwrap_or(300); + + Ok(NormalizedRoutineCreateRequest { + name, + description, + prompt, + trigger, + execution, + delivery, + cooldown_secs, + }) +} + +fn build_routine_trigger(trigger: &NormalizedTriggerRequest) -> Trigger { + match trigger { + NormalizedTriggerRequest::Cron { schedule, timezone } => Trigger::Cron { + schedule: schedule.clone(), + timezone: timezone.clone(), + }, + NormalizedTriggerRequest::Manual => Trigger::Manual, + NormalizedTriggerRequest::MessageEvent { pattern, channel } => Trigger::Event { + channel: channel.clone(), + pattern: pattern.clone(), + }, + NormalizedTriggerRequest::SystemEvent { + source, + event_type, + filters, + } => Trigger::SystemEvent { + source: source.clone(), + event_type: event_type.clone(), + filters: filters.clone(), + }, + } +} + +fn build_routine_action( + name: &str, + prompt: &str, + execution: &NormalizedExecutionRequest, +) -> RoutineAction { + match execution.mode { + NormalizedExecutionMode::Lightweight => RoutineAction::Lightweight { + prompt: prompt.to_string(), + context_paths: execution.context_paths.clone(), + max_tokens: 4096, + use_tools: execution.use_tools, + max_tool_rounds: execution.max_tool_rounds, + }, + NormalizedExecutionMode::FullJob => RoutineAction::FullJob { + title: name.to_string(), + description: prompt.to_string(), + max_iterations: 10, + tool_permissions: execution.tool_permissions.clone(), + }, + } +} + +fn event_emit_schema(include_source_alias: bool) -> Value { + let mut schema = serde_json::json!({ + "type": "object", + "properties": { + "event_source": { + "type": "string", + "description": "Canonical event source, such as 'github'." + }, + "event_type": { + "type": "string", + "description": "Event type, such as 'issue.opened'." + }, + "payload": { + "properties": {}, + "type": "object", + "description": "Structured event payload." + } + }, + "required": ["event_type"] + }); + + if include_source_alias { + if let Some(properties) = schema.get_mut("properties").and_then(Value::as_object_mut) { + properties.insert( + "source".to_string(), + serde_json::json!({ + "type": "string", + "description": "Compatibility alias for event_source." + }), + ); + } + if let Some(schema_obj) = schema.as_object_mut() { + schema_obj.insert( + "anyOf".to_string(), + serde_json::json!([ + { "required": ["event_source"] }, + { "required": ["source"] } + ]), + ); + } + } else if let Some(required) = schema.get_mut("required").and_then(Value::as_array_mut) { + required.push(Value::String("event_source".to_string())); + } + + schema +} + +pub(crate) fn event_emit_parameters_schema() -> Value { + event_emit_schema(false) +} + +fn event_emit_discovery_schema() -> Value { + event_emit_schema(true) +} + +fn parse_event_emit_args(params: &Value) -> Result<(String, String, Value), ToolError> { + let source = params + .get("event_source") + .and_then(Value::as_str) + .or_else(|| params.get("source").and_then(Value::as_str)) + .ok_or_else(|| { + ToolError::InvalidParameters( + "event_emit requires 'event_source' (canonical) or 'source' (alias)".to_string(), + ) + })? + .to_string(); + let event_type = require_str(params, "event_type")?.to_string(); + let payload = params + .get("payload") + .cloned() + .unwrap_or_else(|| serde_json::json!({})); + Ok((source, event_type, payload)) +} pub struct RoutineCreateTool { store: Arc, @@ -179,181 +794,20 @@ impl Tool for RoutineCreateTool { routine_create_parameters_schema() } + fn discovery_schema(&self) -> serde_json::Value { + routine_create_discovery_schema() + } + async fn execute( &self, params: serde_json::Value, ctx: &JobContext, ) -> Result { let start = std::time::Instant::now(); - - let name = require_str(¶ms, "name")?; - - let description = params - .get("description") - .and_then(|v| v.as_str()) - .unwrap_or(""); - - let trigger_type = require_str(¶ms, "trigger_type")?; - - let prompt = require_str(¶ms, "prompt")?; - - // Build trigger - let trigger = match trigger_type { - "cron" => { - let schedule = - params - .get("schedule") - .and_then(|v| v.as_str()) - .ok_or_else(|| { - ToolError::InvalidParameters( - "cron trigger requires 'schedule'".to_string(), - ) - })?; - let timezone = params - .get("timezone") - .and_then(|v| v.as_str()) - .map(|tz| { - crate::timezone::parse_timezone(tz) - .map(|_| tz.to_string()) - .ok_or_else(|| { - ToolError::InvalidParameters(format!( - "invalid IANA timezone: '{tz}'" - )) - }) - }) - .transpose()?; - // Validate cron expression - next_cron_fire(schedule, timezone.as_deref()).map_err(|e| { - ToolError::InvalidParameters(format!("invalid cron schedule: {e}")) - })?; - Trigger::Cron { - schedule: schedule.to_string(), - timezone, - } - } - "event" => { - let pattern = params - .get("event_pattern") - .and_then(|v| v.as_str()) - .ok_or_else(|| { - ToolError::InvalidParameters( - "event trigger requires 'event_pattern'".to_string(), - ) - })?; - // Validate regex with size limit to prevent ReDoS (issue #825) - regex::RegexBuilder::new(pattern) - .size_limit(64 * 1024) - .build() - .map_err(|e| { - ToolError::InvalidParameters(format!("invalid or too complex regex: {e}")) - })?; - let channel = params - .get("event_channel") - .and_then(|v| v.as_str()) - .map(String::from); - Trigger::Event { - channel, - pattern: pattern.to_string(), - } - } - "system_event" => { - let source = params - .get("event_source") - .and_then(|v| v.as_str()) - .ok_or_else(|| { - ToolError::InvalidParameters( - "system_event trigger requires 'event_source'".to_string(), - ) - })?; - let event_type = params - .get("event_type") - .and_then(|v| v.as_str()) - .ok_or_else(|| { - ToolError::InvalidParameters( - "system_event trigger requires 'event_type'".to_string(), - ) - })?; - let filters = params - .get("event_filters") - .and_then(|v| v.as_object()) - .map(|obj| { - obj.iter() - .filter_map(|(k, v)| { - crate::agent::routine::json_value_as_filter_string(v) - .map(|s| (k.to_string(), s)) - }) - .collect::>() - }) - .unwrap_or_default(); - Trigger::SystemEvent { - source: source.to_string(), - event_type: event_type.to_string(), - filters, - } - } - "manual" => Trigger::Manual, - other => { - return Err(ToolError::InvalidParameters(format!( - "unknown trigger_type: {other}" - ))); - } - }; - - // Build action - let action_type = params - .get("action_type") - .and_then(|v| v.as_str()) - .unwrap_or("lightweight"); - - let context_paths: Vec = params - .get("context_paths") - .and_then(|v| v.as_array()) - .map(|arr| { - arr.iter() - .filter_map(|v| v.as_str().map(String::from)) - .collect() - }) - .unwrap_or_default(); - - let use_tools = params - .get("use_tools") - .and_then(|v| v.as_bool()) - .unwrap_or(false); - - let max_tool_rounds = params - .get("max_tool_rounds") - .and_then(|v| v.as_u64()) - .map(|v| v.clamp(1, crate::agent::routine::MAX_TOOL_ROUNDS_LIMIT as u64) as u32) - .unwrap_or(3); - - let action = match action_type { - "lightweight" => RoutineAction::Lightweight { - prompt: prompt.to_string(), - context_paths, - max_tokens: 4096, - use_tools, - max_tool_rounds, - }, - "full_job" => { - let tool_permissions = crate::agent::routine::parse_tool_permissions(¶ms); - RoutineAction::FullJob { - title: name.to_string(), - description: prompt.to_string(), - max_iterations: 10, - tool_permissions, - } - } - other => { - return Err(ToolError::InvalidParameters(format!( - "unknown action_type: {other}" - ))); - } - }; - - let cooldown_secs = params - .get("cooldown_secs") - .and_then(|v| v.as_u64()) - .unwrap_or(300); + let normalized = parse_routine_create_request(¶ms)?; + let trigger = build_routine_trigger(&normalized.trigger); + let action = + build_routine_action(&normalized.name, &normalized.prompt, &normalized.execution); // Compute next fire time for cron let next_fire = if let Trigger::Cron { @@ -368,26 +822,20 @@ impl Tool for RoutineCreateTool { let routine = Routine { id: Uuid::new_v4(), - name: name.to_string(), - description: description.to_string(), + name: normalized.name.clone(), + description: normalized.description.clone(), user_id: ctx.user_id.clone(), enabled: true, trigger, action, guardrails: RoutineGuardrails { - cooldown: Duration::from_secs(cooldown_secs), + cooldown: Duration::from_secs(normalized.cooldown_secs), max_concurrent: 1, dedup_window: None, }, notify: NotifyConfig { - channel: params - .get("notify_channel") - .and_then(|v| v.as_str()) - .map(String::from), - user: params - .get("notify_user") - .and_then(|v| v.as_str()) - .map(String::from), + channel: normalized.delivery.channel.clone(), + user: Some(normalized.delivery.user.clone()), ..NotifyConfig::default() }, last_run_at: None, @@ -522,9 +970,8 @@ impl Tool for RoutineUpdateTool { } fn description(&self) -> &str { - "Update an existing routine. Can change prompt, description, enabled state, or cron timing. \ - Pass the routine name and only the fields you want to change. \ - This does not convert one trigger type into another." + "Update an existing routine. Can change prompt, description, enabled state, or cron schedule/timezone. \ + Pass the routine name and only the fields you want to change. This does not convert trigger types." } fn parameters_schema(&self) -> serde_json::Value { @@ -916,24 +1363,11 @@ impl Tool for EventEmitTool { } fn parameters_schema(&self) -> serde_json::Value { - serde_json::json!({ - "type": "object", - "properties": { - "event_source": { - "type": "string", - "description": "Event source (e.g. 'github', 'workflow', 'tool')" - }, - "event_type": { - "type": "string", - "description": "Event type (e.g. 'issue.opened', 'pr.ready')" - }, - "payload": { - "type": "object", - "description": "Structured event payload" - } - }, - "required": ["event_source", "event_type"] - }) + event_emit_parameters_schema() + } + + fn discovery_schema(&self) -> serde_json::Value { + event_emit_discovery_schema() } async fn execute( @@ -942,22 +1376,16 @@ impl Tool for EventEmitTool { ctx: &JobContext, ) -> Result { let start = std::time::Instant::now(); - - let source = require_str(¶ms, "event_source")?; - let event_type = require_str(¶ms, "event_type")?; - let payload = params - .get("payload") - .cloned() - .unwrap_or_else(|| serde_json::json!({})); + let (source, event_type, payload) = parse_event_emit_args(¶ms)?; let fired = self .engine - .emit_system_event(source, event_type, &payload, Some(&ctx.user_id)) + .emit_system_event(&source, &event_type, &payload, Some(&ctx.user_id)) .await; let result = serde_json::json!({ - "event_source": source, - "event_type": event_type, + "event_source": &source, + "event_type": &event_type, "user_id": &ctx.user_id, "fired_routines": fired, }); @@ -972,81 +1400,432 @@ impl Tool for EventEmitTool { #[cfg(test)] mod tests { - use super::{routine_create_parameters_schema, routine_update_parameters_schema}; + use super::*; use crate::tools::validate_tool_schema; - fn property<'a>(schema: &'a serde_json::Value, name: &str) -> &'a serde_json::Value { + // These tests intentionally use direct assertion macros. + const ROUTINE_CREATE_LEGACY_ALIASES: &[&str] = &[ + "trigger_type", + "schedule", + "timezone", + "event_pattern", + "event_channel", + "event_source", + "event_type", + "event_filters", + "action_type", + "context_paths", + "use_tools", + "max_tool_rounds", + "tool_permissions", + "notify_channel", + "notify_user", + "cooldown_secs", + ]; + + fn schema_property<'a>(schema: &'a Value, name: &str) -> &'a Value { schema .get("properties") - .and_then(|props| props.get(name)) + .and_then(Value::as_object) + .and_then(|properties| properties.get(name)) .unwrap_or_else(|| panic!("missing schema property {name}")) } + fn maybe_schema_property<'a>(schema: &'a Value, name: &str) -> Option<&'a Value> { + schema + .get("properties") + .and_then(Value::as_object) + .and_then(|properties| properties.get(name)) + } + + fn nested_schema_property<'a>(schema: &'a Value, object_name: &str, name: &str) -> &'a Value { + schema_property(schema, object_name) + .get("properties") + .and_then(Value::as_object) + .and_then(|properties| properties.get(name)) + .unwrap_or_else(|| panic!("missing nested schema property {object_name}.{name}")) + } + + #[test] + fn parses_grouped_manual_lightweight_request() { + let params = serde_json::json!({ + "name": "manual-check", + "prompt": "Inspect the repo for issues.", + "request": { + "kind": "manual" + } + }); + + let parsed = parse_routine_create_request(¶ms).expect("parse grouped manual request"); + + assert_eq!(parsed.name.as_str(), "manual-check"); + assert_eq!(parsed.prompt.as_str(), "Inspect the repo for issues."); + assert!( + matches!(parsed.trigger, NormalizedTriggerRequest::Manual), + "expected manual trigger", + ); + assert!( + matches!(parsed.execution.mode, NormalizedExecutionMode::Lightweight), + "expected lightweight execution mode", + ); + assert_eq!(parsed.cooldown_secs, 300); + assert_eq!(parsed.delivery.user.as_str(), "default"); + } + + #[test] + fn parses_grouped_cron_full_job_request() { + let params = serde_json::json!({ + "name": "weekday-digest", + "prompt": "Prepare the morning digest.", + "request": { + "kind": "cron", + "schedule": "0 0 9 * * MON-FRI", + "timezone": "UTC" + }, + "execution": { + "mode": "full_job", + "tool_permissions": ["message", "http"] + }, + "delivery": { + "channel": "telegram", + "user": "ops-team" + }, + "advanced": { + "cooldown_secs": 30 + } + }); + + let parsed = parse_routine_create_request(¶ms).expect("parse grouped cron request"); + + assert!( + matches!( + parsed.trigger, + NormalizedTriggerRequest::Cron { ref schedule, ref timezone } + if schedule == "0 0 9 * * MON-FRI" && timezone.as_deref() == Some("UTC") + ), + "expected grouped cron trigger", + ); + assert!( + matches!(parsed.execution.mode, NormalizedExecutionMode::FullJob), + "expected full_job execution mode", + ); + assert_eq!( + parsed.execution.tool_permissions, + vec!["message".to_string(), "http".to_string()], + ); + assert_eq!(parsed.delivery.channel.as_deref(), Some("telegram")); + assert_eq!(parsed.delivery.user.as_str(), "ops-team"); + assert_eq!(parsed.cooldown_secs, 30); + } + #[test] - fn routine_create_schema_exposes_all_trigger_and_delivery_fields() { + fn parses_grouped_message_event_with_tools() { + let params = serde_json::json!({ + "name": "deploy-watch", + "prompt": "Look for deploy requests.", + "request": { + "kind": "message_event", + "pattern": "deploy\\s+prod", + "channel": "slack" + }, + "execution": { + "use_tools": true, + "max_tool_rounds": 5, + "context_paths": ["context/deploy.md"] + } + }); + + let parsed = + parse_routine_create_request(¶ms).expect("parse grouped message event request"); + + assert!( + matches!( + parsed.trigger, + NormalizedTriggerRequest::MessageEvent { ref pattern, ref channel } + if pattern == "deploy\\s+prod" && channel.as_deref() == Some("slack") + ), + "expected grouped message_event trigger", + ); + assert!(parsed.execution.use_tools, "expected use_tools=true"); + assert_eq!(parsed.execution.max_tool_rounds, 5); + assert_eq!( + parsed.execution.context_paths, + vec!["context/deploy.md".to_string()], + ); + } + + #[test] + fn parses_grouped_system_event_request() { + let params = serde_json::json!({ + "name": "issue-watch", + "prompt": "Summarize new GitHub issues.", + "request": { + "kind": "system_event", + "source": "github", + "event_type": "issue.opened", + "filters": { + "repository": "nearai/ironclaw", + "public": true, + "issue_number": 42 + } + }, + "execution": { + "mode": "full_job" + } + }); + + let parsed = + parse_routine_create_request(¶ms).expect("parse grouped system event request"); + + assert!( + matches!( + parsed.trigger, + NormalizedTriggerRequest::SystemEvent { ref source, ref event_type, ref filters } + if source == "github" + && event_type == "issue.opened" + && filters.get("repository") == Some(&"nearai/ironclaw".to_string()) + && filters.get("public") == Some(&"true".to_string()) + && filters.get("issue_number") == Some(&"42".to_string()) + ), + "expected grouped system_event trigger", + ); + } + + #[test] + fn rejects_system_event_filters_with_nested_values() { + let params = serde_json::json!({ + "name": "issue-watch", + "prompt": "Summarize new GitHub issues.", + "request": { + "kind": "system_event", + "source": "github", + "event_type": "issue.opened", + "filters": { + "repository": { + "owner": "nearai", + "name": "ironclaw" + } + } + } + }); + + let err = parse_routine_create_request(¶ms) + .expect_err("reject nested system event filter values"); + match err { + ToolError::InvalidParameters(message) => { + assert!( + message.contains( + "system_event filters only support string, number, and boolean values", + ), + "unexpected invalid filter error: {message}", + ) + } + other => panic!("expected InvalidParameters, got {other:?}"), + } + } + + #[test] + fn parses_legacy_flat_shape() { + let params = serde_json::json!({ + "name": "legacy-routine", + "prompt": "Legacy create path.", + "trigger_type": "event", + "event_pattern": "hello", + "event_channel": "telegram", + "action_type": "full_job", + "tool_permissions": ["message"], + "notify_channel": "telegram", + "notify_user": "123" + }); + + let parsed = parse_routine_create_request(¶ms).expect("parse legacy flat request"); + + assert!( + matches!( + parsed.trigger, + NormalizedTriggerRequest::MessageEvent { ref pattern, ref channel } + if pattern == "hello" && channel.as_deref() == Some("telegram") + ), + "expected legacy message_event trigger", + ); + assert!( + matches!(parsed.execution.mode, NormalizedExecutionMode::FullJob), + "expected full_job execution mode", + ); + assert_eq!( + parsed.execution.tool_permissions, + vec!["message".to_string()], + ); + assert_eq!(parsed.delivery.channel.as_deref(), Some("telegram")); + assert_eq!(parsed.delivery.user.as_str(), "123"); + } + + #[test] + fn parses_mixed_grouped_and_legacy_aliases() { + let params = serde_json::json!({ + "name": "mixed-routine", + "prompt": "Mixed payload.", + "request": { + "kind": "cron" + }, + "schedule": "0 0 8 * * *", + "timezone": "UTC", + "execution": { + "mode": "lightweight" + }, + "notify_user": "fallback-user", + "advanced": { + "cooldown_secs": 45 + } + }); + + let parsed = parse_routine_create_request(¶ms).expect("parse mixed request"); + + assert!( + matches!( + parsed.trigger, + NormalizedTriggerRequest::Cron { ref schedule, ref timezone } + if schedule == "0 0 8 * * *" && timezone.as_deref() == Some("UTC") + ), + "expected mixed cron trigger", + ); + assert_eq!(parsed.delivery.user.as_str(), "fallback-user"); + assert_eq!(parsed.cooldown_secs, 45); + } + + #[test] + fn parses_event_emit_with_source_alias() { + let params = serde_json::json!({ + "source": "github", + "event_type": "issue.opened", + "payload": { "issue_number": 7 } + }); + + let (source, event_type, payload) = + parse_event_emit_args(¶ms).expect("parse event_emit source alias"); + + assert_eq!(source, "github".to_string()); + assert_eq!(event_type, "issue.opened".to_string()); + assert_eq!(payload["issue_number"].clone(), serde_json::json!(7)); + } + + #[test] + fn parses_event_emit_with_event_source() { + let params = serde_json::json!({ + "event_source": "github", + "event_type": "issue.opened" + }); + + let (source, event_type, payload) = + parse_event_emit_args(¶ms).expect("parse canonical event_emit args"); + + assert_eq!(source, "github".to_string()); + assert_eq!(event_type, "issue.opened".to_string()); + assert_eq!(payload, serde_json::json!({})); + } + + #[test] + fn routine_create_parameters_schema_prefers_grouped_request_shape() { let schema = routine_create_parameters_schema(); let errors = validate_tool_schema(&schema, "routine_create"); assert!( errors.is_empty(), - "routine_create schema should validate cleanly: {errors:?}" + "routine_create schema should validate cleanly: {errors:?}", ); - for field in [ - "trigger_type", - "schedule", - "event_pattern", - "event_channel", - "event_source", - "event_type", - "event_filters", - "action_type", - "use_tools", - "max_tool_rounds", - "tool_permissions", - "notify_channel", - "notify_user", - "timezone", - ] { - let _ = property(&schema, field); + let request = schema_property(&schema, "request"); + assert!( + request.is_object(), + "request should be present in compact schema", + ); + let required = schema + .get("required") + .and_then(Value::as_array) + .expect("routine_create required list"); + assert!( + required.contains(&Value::String("request".to_string())), + "compact parameters schema should require request", + ); + + for legacy_alias in ROUTINE_CREATE_LEGACY_ALIASES { + assert!( + maybe_schema_property(&schema, legacy_alias).is_none(), + "compact parameters schema should hide legacy alias", + ); } } #[test] - fn routine_create_schema_descriptions_cover_event_trigger_gotchas() { + fn routine_create_discovery_schema_keeps_legacy_aliases() { + let schema = routine_create_discovery_schema(); + let any_of = schema + .get("anyOf") + .and_then(Value::as_array) + .expect("routine_create discovery anyOf"); + assert_eq!(any_of.len(), 2usize); + + for legacy_alias in ROUTINE_CREATE_LEGACY_ALIASES { + assert!( + schema_property(&schema, legacy_alias).is_object(), + "discovery schema should retain legacy alias", + ); + } + } + + #[test] + fn routine_create_parameters_schema_describes_grouped_trigger_fields() { let schema = routine_create_parameters_schema(); - let trigger_type = property(&schema, "trigger_type") + let request_description = schema_property(&schema, "request") .get("description") - .and_then(|value| value.as_str()) - .expect("trigger_type description"); - assert!(trigger_type.contains("incoming messages")); - assert!(trigger_type.contains("structured emitted events")); + .and_then(Value::as_str) + .expect("request description"); + assert!( + request_description.contains("Set request.kind first"), + "request description should mention kind-first guidance", + ); - let event_pattern = property(&schema, "event_pattern") + let pattern_description = nested_schema_property(&schema, "request", "pattern") .get("description") - .and_then(|value| value.as_str()) - .expect("event_pattern description"); - assert!(event_pattern.contains("incoming message text")); - assert!(event_pattern.contains("^bug\\\\b")); + .and_then(Value::as_str) + .expect("request.pattern description"); + assert!( + pattern_description.contains("message_event"), + "pattern description should mention message_event", + ); - let event_channel = property(&schema, "event_channel") + let source_description = nested_schema_property(&schema, "request", "source") .get("description") - .and_then(|value| value.as_str()) - .expect("event_channel description"); - assert!(event_channel.contains("Omit to match any channel")); - assert!(event_channel.contains("Not a chat or thread ID")); + .and_then(Value::as_str) + .expect("request.source description"); + assert!( + source_description.contains("system_event"), + "source description should mention system_event", + ); - let notify_channel = property(&schema, "notify_channel") + let filters_description = nested_schema_property(&schema, "request", "filters") .get("description") - .and_then(|value| value.as_str()) - .expect("notify_channel description"); - assert!(notify_channel.contains("does not control what triggers")); + .and_then(Value::as_str) + .expect("request.filters description"); + assert!( + filters_description.contains("top-level string, number, and boolean"), + "filters description should mention supported scalar payload types", + ); - let prompt = property(&schema, "prompt") - .get("description") - .and_then(|value| value.as_str()) - .expect("prompt description"); - assert!(prompt.contains("after it fires")); + let filters_schema = nested_schema_property(&schema, "request", "filters"); + let additional_properties = filters_schema + .get("additionalProperties") + .expect("request.filters additionalProperties"); + let allowed_types = additional_properties + .get("type") + .and_then(Value::as_array) + .expect("request.filters additionalProperties.type"); + assert!( + allowed_types.contains(&Value::String("string".to_string())) + && allowed_types.contains(&Value::String("number".to_string())) + && allowed_types.contains(&Value::String("boolean".to_string())), + "filters schema should constrain additionalProperties to scalar values", + ); } #[test] @@ -1055,7 +1834,7 @@ mod tests { let errors = validate_tool_schema(&schema, "routine_update"); assert!( errors.is_empty(), - "routine_update schema should validate cleanly: {errors:?}" + "routine_update schema should validate cleanly: {errors:?}", ); for field in [ @@ -1066,20 +1845,66 @@ mod tests { "timezone", "description", ] { - let _ = property(&schema, field); + let _ = schema_property(&schema, field); } - let schedule = property(&schema, "schedule") + let schedule_description = schema_property(&schema, "schedule") .get("description") - .and_then(|value| value.as_str()) + .and_then(Value::as_str) .expect("schedule description"); - assert!(schedule.contains("existing 'cron' routines only")); - assert!(schedule.contains("does not convert other trigger types")); + assert!( + schedule_description.contains("cron triggers"), + "schedule description should mention cron triggers", + ); - let timezone = property(&schema, "timezone") + let timezone_description = schema_property(&schema, "timezone") .get("description") - .and_then(|value| value.as_str()) + .and_then(Value::as_str) .expect("timezone description"); - assert!(timezone.contains("existing 'cron' routines only")); + assert!( + timezone_description.contains("cron triggers"), + "timezone description should mention cron triggers", + ); + } + + #[test] + fn event_emit_parameters_schema_prefers_canonical_event_source() { + let schema = event_emit_parameters_schema(); + let errors = validate_tool_schema(&schema, "event_emit"); + assert!( + errors.is_empty(), + "event_emit schema should validate cleanly: {errors:?}", + ); + + assert!( + schema_property(&schema, "event_source").is_object(), + "event_emit parameters schema should expose event_source", + ); + let required = schema + .get("required") + .and_then(Value::as_array) + .expect("event_emit required list"); + assert!( + required.contains(&Value::String("event_source".to_string())), + "event_emit parameters schema should require event_source", + ); + assert!( + maybe_schema_property(&schema, "source").is_none(), + "event_emit parameters schema should hide source alias", + ); + } + + #[test] + fn event_emit_discovery_schema_keeps_source_alias() { + let schema = event_emit_discovery_schema(); + let any_of = schema + .get("anyOf") + .and_then(Value::as_array) + .expect("event_emit discovery anyOf"); + assert_eq!(any_of.len(), 2usize); + assert!( + schema_property(&schema, "source").is_object(), + "event_emit discovery schema should keep source alias", + ); } } diff --git a/src/tools/schema_validator.rs b/src/tools/schema_validator.rs index 9cc2fa5f3..df87afa4e 100644 --- a/src/tools/schema_validator.rs +++ b/src/tools/schema_validator.rs @@ -605,15 +605,7 @@ mod tests { ), ( "event_emit", - serde_json::json!({ - "type": "object", - "properties": { - "event_source": { "type": "string", "description": "Event source" }, - "event_type": { "type": "string", "description": "Event type" }, - "payload": { "type": "object", "description": "Event payload", "properties": {} } - }, - "required": ["event_source", "event_type"] - }), + crate::tools::builtin::routine::event_emit_parameters_schema(), ), // Job tools with complex deps ( diff --git a/tests/e2e_builtin_tool_coverage.rs b/tests/e2e_builtin_tool_coverage.rs index 2a97a0d50..08d56d4f2 100644 --- a/tests/e2e_builtin_tool_coverage.rs +++ b/tests/e2e_builtin_tool_coverage.rs @@ -142,16 +142,18 @@ mod tests { match &routine.action { RoutineAction::Lightweight { + prompt, context_paths, use_tools, max_tool_rounds, .. } => { + assert!(prompt.contains("Check system status")); assert_eq!(context_paths, &vec!["context/priorities.md".to_string()]); assert!(*use_tools, "lightweight routine should keep use_tools=true"); assert_eq!(*max_tool_rounds, 2); } - other => panic!("expected lightweight action, got {other:?}"), + other => panic!("expected lightweight routine action, got {other:?}"), } assert_eq!(routine.notify.channel.as_deref(), Some("telegram")); @@ -369,7 +371,132 @@ mod tests { } // ----------------------------------------------------------------------- - // Test 8: skill_install_routine_webhook_sim + // Test 8: routine_create_grouped + // ----------------------------------------------------------------------- + + #[tokio::test] + async fn routine_create_grouped() { + let trace = LlmTrace::from_file(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/fixtures/llm_traces/tools/routine_create_grouped.json" + )) + .expect("failed to load routine_create_grouped.json"); + + let rig = TestRigBuilder::new() + .with_trace(trace.clone()) + .with_auto_approve_tools(true) + .build() + .await; + + rig.send_message("Create a grouped cron routine with delivery settings") + .await; + let responses = rig.wait_for_responses(1, Duration::from_secs(15)).await; + + rig.verify_trace_expects(&trace, &responses); + + let routine = rig + .database() + .get_routine_by_name("test-user", "weekday-digest") + .await + .expect("get_routine_by_name") + .expect("weekday-digest should exist"); + + match &routine.trigger { + Trigger::Cron { schedule, timezone } => { + assert_eq!(schedule, "0 0 9 * * MON-FRI"); + assert_eq!(timezone.as_deref(), Some("UTC")); + } + other => panic!("expected cron trigger, got {other:?}"), + } + + match &routine.action { + RoutineAction::FullJob { + description, + tool_permissions, + .. + } => { + assert!(description.contains("Prepare the morning digest")); + assert_eq!( + tool_permissions, + &vec!["message".to_string(), "http".to_string()] + ); + } + other => panic!("expected full_job action, got {other:?}"), + } + + assert_eq!(routine.notify.channel.as_deref(), Some("telegram")); + assert_eq!(routine.notify.user, "ops-team"); + assert_eq!(routine.guardrails.cooldown.as_secs(), 30); + + rig.shutdown(); + } + + // ----------------------------------------------------------------------- + // Test 9: routine_system_event_emit_grouped + // ----------------------------------------------------------------------- + + #[tokio::test] + async fn routine_system_event_emit_grouped() { + let trace = LlmTrace::from_file(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/fixtures/llm_traces/tools/routine_system_event_emit_grouped.json" + )) + .expect("failed to load routine_system_event_emit_grouped.json"); + + let rig = TestRigBuilder::new() + .with_trace(trace.clone()) + .with_auto_approve_tools(true) + .build() + .await; + + rig.send_message("Create a grouped system-event routine and emit a matching event") + .await; + let responses = rig.wait_for_responses(1, Duration::from_secs(15)).await; + + rig.verify_trace_expects(&trace, &responses); + + let routine = rig + .database() + .get_routine_by_name("test-user", "grouped-gh-issue-watch") + .await + .expect("get_routine_by_name") + .expect("grouped-gh-issue-watch should exist"); + + match &routine.trigger { + Trigger::SystemEvent { + source, + event_type, + filters, + } => { + assert_eq!(source, "github"); + assert_eq!(event_type, "issue.opened"); + assert_eq!( + filters.get("repository").map(String::as_str), + Some("nearai/ironclaw") + ); + assert_eq!(filters.get("priority").map(String::as_str), Some("p1")); + } + other => panic!("expected system_event trigger, got {other:?}"), + } + + let results = rig.tool_results(); + let emit_result = results + .iter() + .find(|(n, _)| n == "event_emit") + .expect("event_emit result missing"); + let emit_json: serde_json::Value = + serde_json::from_str(&emit_result.1).expect("event_emit result should be valid JSON"); + assert!( + emit_json["fired_routines"].as_u64().unwrap_or(0) > 0, + "event_emit should have fired at least one grouped routine: {:?}", + emit_result.1 + ); + + rig.shutdown(); + } + + // ----------------------------------------------------------------------- + // Test 10: skill_install_routine_webhook_sim // ----------------------------------------------------------------------- #[tokio::test] diff --git a/tests/fixtures/llm_traces/tools/routine_create_grouped.json b/tests/fixtures/llm_traces/tools/routine_create_grouped.json new file mode 100644 index 000000000..ae4b6eb95 --- /dev/null +++ b/tests/fixtures/llm_traces/tools/routine_create_grouped.json @@ -0,0 +1,66 @@ +{ + "model_name": "test-routine-create-grouped", + "expects": { + "tools_used": ["routine_create", "routine_list"], + "all_tools_succeeded": true, + "min_responses": 1 + }, + "steps": [ + { + "response": { + "type": "tool_calls", + "tool_calls": [ + { + "id": "call_rc_grouped_1", + "name": "routine_create", + "arguments": { + "name": "weekday-digest", + "prompt": "Prepare the morning digest for the ops team.", + "description": "Weekday digest for morning operations", + "request": { + "kind": "cron", + "schedule": "0 0 9 * * MON-FRI", + "timezone": "UTC" + }, + "execution": { + "mode": "full_job", + "tool_permissions": ["message", "http"] + }, + "delivery": { + "channel": "telegram", + "user": "ops-team" + }, + "advanced": { + "cooldown_secs": 30 + } + } + } + ], + "input_tokens": 130, + "output_tokens": 44 + } + }, + { + "response": { + "type": "tool_calls", + "tool_calls": [ + { + "id": "call_rl_grouped_1", + "name": "routine_list", + "arguments": {} + } + ], + "input_tokens": 190, + "output_tokens": 20 + } + }, + { + "response": { + "type": "text", + "content": "Created the weekday-digest routine with a grouped cron request and listed the active routines.", + "input_tokens": 250, + "output_tokens": 24 + } + } + ] +} diff --git a/tests/fixtures/llm_traces/tools/routine_system_event_emit_grouped.json b/tests/fixtures/llm_traces/tools/routine_system_event_emit_grouped.json new file mode 100644 index 000000000..61f159c0a --- /dev/null +++ b/tests/fixtures/llm_traces/tools/routine_system_event_emit_grouped.json @@ -0,0 +1,74 @@ +{ + "model_name": "test-routine-system-event-emit-grouped", + "expects": { + "tools_used": ["routine_create", "event_emit"], + "all_tools_succeeded": true, + "tool_results_contain": { + "event_emit": "fired_routines" + } + }, + "steps": [ + { + "response": { + "type": "tool_calls", + "tool_calls": [ + { + "id": "call_rc_grouped_system_1", + "name": "routine_create", + "arguments": { + "name": "grouped-gh-issue-watch", + "prompt": "Summarize the new issue and propose next steps.", + "description": "React to important GitHub issue.opened events", + "request": { + "kind": "system_event", + "source": "github", + "event_type": "issue.opened", + "filters": { + "repository": "nearai/ironclaw", + "priority": "p1" + } + }, + "execution": { + "mode": "full_job", + "tool_permissions": ["shell"] + } + } + } + ], + "input_tokens": 120, + "output_tokens": 40 + } + }, + { + "response": { + "type": "tool_calls", + "tool_calls": [ + { + "id": "call_ee_grouped_1", + "name": "event_emit", + "arguments": { + "event_source": "github", + "event_type": "issue.opened", + "payload": { + "repository": "nearai/ironclaw", + "priority": "p1", + "issue_number": 123, + "title": "Support grouped routine create requests" + } + } + } + ], + "input_tokens": 180, + "output_tokens": 30 + } + }, + { + "response": { + "type": "text", + "content": "Created the grouped system-event routine and emitted a matching GitHub event.", + "input_tokens": 230, + "output_tokens": 18 + } + } + ] +}