diff --git a/skills/ironclaw-workflow-orchestrator/references/workflow-routines.md b/skills/ironclaw-workflow-orchestrator/references/workflow-routines.md index 8afa857d7c..5e64a2b2ca 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/agent/routine_engine.rs b/src/agent/routine_engine.rs index 519f16c22a..bf044139e3 100644 --- a/src/agent/routine_engine.rs +++ b/src/agent/routine_engine.rs @@ -784,23 +784,12 @@ async fn execute_lightweight( Err(_) => None, }; - // Build the user-facing prompt - let mut full_prompt = String::new(); - full_prompt.push_str(prompt); - - if !context_parts.is_empty() { - full_prompt.push_str("\n\n---\n\n# Context\n\n"); - full_prompt.push_str(&context_parts.join("\n\n")); - } - - if let Some(state) = &state_content { - full_prompt.push_str("\n\n---\n\n# Previous State\n\n"); - full_prompt.push_str(state); - } - - full_prompt.push_str( - "\n\n---\n\nIf nothing needs attention, reply EXACTLY with: ROUTINE_OK\n\ - If something needs attention, provide a concise summary.", + let full_prompt = build_lightweight_prompt( + prompt, + &context_parts, + state_content.as_deref(), + &routine.notify, + use_tools, ); // Get system prompt @@ -844,6 +833,65 @@ async fn execute_lightweight( } } +fn build_lightweight_prompt( + prompt: &str, + context_parts: &[String], + state_content: Option<&str>, + notify: &NotifyConfig, + use_tools: bool, +) -> String { + let mut full_prompt = String::new(); + full_prompt.push_str(prompt); + + if notify.on_attention { + full_prompt.push_str("\n\n---\n\n# Delivery\n\n"); + full_prompt.push_str( + "If you reply with anything other than ROUTINE_OK, the host will deliver your \ + reply as the routine notification. Return the message exactly as it should be sent.\n", + ); + + if let Some(channel) = notify.channel.as_deref() { + full_prompt.push_str(&format!( + "The configured delivery channel for this routine is `{channel}`.\n" + )); + } + + if let Some(user) = notify.user.as_deref() { + full_prompt.push_str(&format!( + "The configured delivery target for this routine is `{user}`.\n" + )); + } + + full_prompt.push_str( + "Do not claim you lack messaging integrations or ask the user to set one up when \ + a plain reply is sufficient.\n", + ); + } + + if !use_tools { + full_prompt.push_str( + "\nTools are disabled for this routine run. Do not ask to call tools or describe tool limitations unless they prevent a necessary external action.\n", + ); + } + + if !context_parts.is_empty() { + full_prompt.push_str("\n\n---\n\n# Context\n\n"); + full_prompt.push_str(&context_parts.join("\n\n")); + } + + if let Some(state) = state_content { + full_prompt.push_str("\n\n---\n\n# Previous State\n\n"); + full_prompt.push_str(state); + } + + full_prompt.push_str( + "\n\n---\n\nIf nothing needs attention, reply EXACTLY with: ROUTINE_OK\n\ + If something needs attention, provide a concise summary.", + ); + + full_prompt +} + /// Execute a lightweight routine without tool support (original single-call behavior). async fn execute_lightweight_no_tools( ctx: &EngineContext, @@ -1385,6 +1433,61 @@ mod tests { } } + #[test] + fn test_build_lightweight_prompt_explains_delivery_and_disabled_tools() { + let notify = NotifyConfig { + channel: Some("telegram".to_string()), + user: Some("default".to_string()), + on_attention: true, + on_failure: true, + on_success: false, + }; + + let prompt = super::build_lightweight_prompt( + "Send a Telegram reminder message to the user.", + &[], + None, + ¬ify, + false, + ); + + assert!( + prompt.contains("the host will deliver your reply as the routine notification"), + "delivery guidance should explain host delivery: {prompt}", + ); + assert!( + prompt.contains("configured delivery channel for this routine is `telegram`"), + "delivery guidance should mention telegram channel: {prompt}", + ); + assert!( + prompt.contains("Do not claim you lack messaging integrations"), + "delivery guidance should suppress fake setup chatter: {prompt}", + ); + assert!( + prompt.contains("Tools are disabled for this routine run"), + "prompt should explain that tools are disabled: {prompt}", + ); + } + + #[test] + fn test_build_lightweight_prompt_skips_delivery_block_when_attention_notifications_disabled() { + let notify = NotifyConfig { + on_attention: false, + ..NotifyConfig::default() + }; + + let prompt = super::build_lightweight_prompt("Check inbox.", &[], None, ¬ify, true); + + assert!( + !prompt.contains("# Delivery"), + "prompt should not include delivery guidance when attention notifications are off: {prompt}", + ); + assert!( + !prompt.contains("Tools are disabled for this routine run"), + "prompt should not claim tools are disabled when they are enabled: {prompt}", + ); + } + #[test] fn test_routine_sentinel_detection_exact_match() { // The execute_lightweight_no_tools checks: content == "ROUTINE_OK" || content.contains("ROUTINE_OK") diff --git a/src/tools/builtin/routine.rs b/src/tools/builtin/routine.rs index 347cb4ff07..bf1c0d5797 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::{ @@ -22,135 +24,1010 @@ use crate::agent::routine::{ use crate::agent::routine_engine::RoutineEngine; use crate::context::JobContext; use crate::db::Database; -use crate::tools::tool::{ApprovalRequirement, Tool, ToolError, ToolOutput, require_str}; +use crate::tools::tool::{ + ApprovalRequirement, Tool, ToolDiscoverySummary, ToolError, ToolOutput, require_str, +}; + +// ==================== 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: Option, +} + +#[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. If omitted, the owner's last-seen notification target is used." + } + }) +} + +fn advanced_properties() -> Value { + serde_json::json!({ + "cooldown_secs": { + "type": "integer", + "description": "Minimum seconds between automatic fires. Manual fires still bypass cooldown." + } + }) +} -pub(crate) fn routine_create_parameters_schema() -> serde_json::Value { +fn manual_request_variant() -> Value { serde_json::json!({ "type": "object", + "description": "Manual routines run only when explicitly fired.", "properties": { - "name": { + "kind": { "type": "string", - "description": "Unique routine name, for example 'daily-pr-review'." - }, - "description": { + "enum": ["manual"], + "description": "Manual trigger." + } + }, + "required": ["kind"] + }) +} + +fn cron_request_variant() -> Value { + serde_json::json!({ + "type": "object", + "description": "Cron routines require request.schedule and may optionally set request.timezone.", + "properties": { + "kind": { "type": "string", - "description": "Short summary of what the routine is for." + "enum": ["cron"], + "description": "Scheduled trigger." }, - "trigger_type": { + "schedule": { "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." + "description": "Cron expression for request.kind='cron'. Uses 6-field cron: second minute hour day month weekday." }, - "schedule": { + "timezone": { + "type": "string", + "description": "IANA timezone for request.kind='cron', such as 'America/New_York'." + } + }, + "required": ["kind", "schedule"] + }) +} + +fn message_event_request_variant() -> Value { + serde_json::json!({ + "type": "object", + "description": "Message-event routines require request.pattern and may optionally filter by request.channel.", + "properties": { + "kind": { "type": "string", - "description": "Cron schedule for 'cron' triggers. Uses 6 fields: second minute hour day month weekday." + "enum": ["message_event"], + "description": "Pattern-matching message trigger." }, - "event_pattern": { + "pattern": { "type": "string", - "description": "Regex matched against incoming message text for 'event' triggers, for example '^bug\\\\b'." + "description": "Regex pattern for request.kind='message_event'." }, - "event_channel": { + "channel": { + "type": "string", + "description": "Optional channel filter for request.kind='message_event'." + } + }, + "required": ["kind", "pattern"] + }) +} + +fn system_event_request_variant() -> Value { + serde_json::json!({ + "type": "object", + "description": "System-event routines require request.source and request.event_type. request.filters is optional.", + "properties": { + "kind": { "type": "string", - "description": "Optional platform filter for 'event' triggers, for example 'telegram'. Omit to match any channel. Not a chat or thread ID." + "enum": ["system_event"], + "description": "Structured event trigger." }, - "event_source": { + "source": { "type": "string", - "description": "Structured event source for 'system_event' triggers, for example 'github'." + "description": "Event source namespace for request.kind='system_event', such as 'github'." }, "event_type": { "type": "string", - "description": "Structured event type for 'system_event' triggers, for example 'issue.opened'." + "description": "Event type for request.kind='system_event', such as 'issue.opened'." }, - "event_filters": { + "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." - }, - "prompt": { + "description": "Optional exact-match filters for request.kind='system_event'. Only top-level string, number, and boolean payload fields are matched." + } + }, + "required": ["kind", "source", "event_type"] + }) +} + +fn routine_request_discovery_schema() -> Value { + serde_json::json!({ + "type": "object", + "description": "Canonical trigger config. Set request.kind first, then follow the matching variant branch below.", + "properties": routine_request_properties(), + "required": ["kind"], + "oneOf": [ + manual_request_variant(), + cron_request_variant(), + message_event_request_variant(), + system_event_request_variant() + ], + "examples": [ + { "kind": "manual" }, + { "kind": "cron", "schedule": "0 0 9 * * MON-FRI", "timezone": "UTC" }, + { "kind": "message_event", "pattern": "deploy\\s+prod", "channel": "slack" }, + { "kind": "system_event", "source": "github", "event_type": "issue.opened", "filters": { "repository": "nearai/ironclaw" } } + ] + }) +} + +fn lightweight_execution_variant() -> Value { + serde_json::json!({ + "type": "object", + "description": "Default lightweight execution. Applies when execution is omitted or execution.mode='lightweight'.", + "properties": { + "mode": { "type": "string", - "description": "Instructions for what the routine should do after it fires." + "enum": ["lightweight"], + "description": "Lightweight execution mode." }, "context_paths": { "type": "array", "items": { "type": "string" }, - "description": "Workspace paths to load as extra context before running the routine." - }, - "action_type": { - "type": "string", - "enum": ["lightweight", "full_job"], - "description": "Execution mode: 'lightweight' for one LLM turn or 'full_job' for a multi-step job with tools." + "description": "Workspace paths to preload for lightweight routines." }, "use_tools": { "type": "boolean", - "description": "Enable safe tool use in 'lightweight' mode. Ignored for 'full_job'." + "description": "When true, safe non-approval tools are available." }, "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." + "minimum": 1, + "maximum": crate::agent::routine::MAX_TOOL_ROUNDS_LIMIT, + "default": 3, + "description": "Only applies when use_tools=true. Runtime-capped to prevent loops." + } + } + }) +} + +fn full_job_execution_variant() -> Value { + serde_json::json!({ + "type": "object", + "description": "Full-job execution. Uses tool_permissions and ignores lightweight-only fields such as use_tools, max_tool_rounds, and context_paths.", + "properties": { + "mode": { + "type": "string", + "enum": ["full_job"], + "description": "Full-job execution mode." }, "tool_permissions": { "type": "array", "items": { "type": "string" }, - "description": "Pre-authorized tool names for 'full_job' routines." + "description": "Tools pre-authorized for Always-approval checks." + } + }, + "required": ["mode"] + }) +} + +fn execution_discovery_schema() -> Value { + serde_json::json!({ + "type": "object", + "description": "Optional execution settings. Omit this block for the default lightweight mode.", + "properties": execution_properties(), + "oneOf": [ + lightweight_execution_variant(), + full_job_execution_variant() + ], + "examples": [ + { "mode": "lightweight", "use_tools": true, "max_tool_rounds": 3 }, + { "mode": "full_job", "tool_permissions": ["message", "http"] } + ] + }) +} + +fn routine_create_examples() -> Vec { + vec![ + serde_json::json!({ + "name": "manual-check", + "prompt": "Inspect the repo for issues.", + "request": { "kind": "manual" } + }), + serde_json::json!({ + "name": "weekday-digest", + "prompt": "Prepare the morning digest.", + "request": { + "kind": "cron", + "schedule": "0 0 9 * * MON-FRI", + "timezone": "UTC" + }, + "delivery": { + "channel": "telegram", + "user": "ops-team" + } + }), + serde_json::json!({ + "name": "deploy-watch", + "prompt": "Look for deploy requests.", + "request": { + "kind": "message_event", + "pattern": "deploy\\s+prod", + "channel": "slack" }, - "notify_channel": { + "execution": { + "mode": "lightweight", + "use_tools": true, + "max_tool_rounds": 5 + } + }), + 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" } + }, + "execution": { + "mode": "full_job", + "tool_permissions": ["message"] + } + }), + ] +} + +fn routine_create_tool_summary() -> ToolDiscoverySummary { + ToolDiscoverySummary { + always_required: vec!["name".into(), "prompt".into(), "request.kind".into()], + conditional_requirements: vec![ + "request.kind='cron' requires request.schedule.".into(), + "request.kind='message_event' requires request.pattern.".into(), + "request.kind='system_event' requires request.source and request.event_type.".into(), + "execution.mode='full_job' uses tool_permissions and ignores use_tools, max_tool_rounds, and context_paths.".into(), + ], + notes: vec![ + "Omitting execution defaults to lightweight mode.".into(), + "Omitting delivery.user falls back to the owner's last-seen notification target.".into(), + "advanced.cooldown_secs defaults to 300.".into(), + "Legacy flat aliases are still accepted for compatibility, but grouped fields are preferred.".into(), + ], + examples: routine_create_examples(), + } +} + +fn routine_create_schema(include_compatibility_aliases: bool) -> Value { + let mut schema = serde_json::json!({ + "type": "object", + "properties": { + "name": { "type": "string", - "description": "Where routine output should be sent, for example 'telegram' or 'slack'. This does not control what triggers the routine." + "description": "Unique name for the routine (e.g. 'daily-pr-review')." }, - "notify_user": { + "prompt": { "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." + "description": "Instructions for what the routine should do when it fires." }, - "timezone": { + "description": { "type": "string", - "description": "IANA timezone used to evaluate 'cron' schedules, for example 'America/New_York'." + "description": "Optional human-readable summary of what the routine does." + }, + "request": if include_compatibility_aliases { + routine_request_discovery_schema() + } else { + serde_json::json!({ + "type": "object", + "description": "Canonical trigger config. Set request.kind first, then only fill fields that match that kind.", + "properties": routine_request_properties(), + "required": ["kind"] + }) + }, + "execution": if include_compatibility_aliases { + execution_discovery_schema() + } else { + serde_json::json!({ + "type": "object", + "description": "Optional execution settings. Omit for the default lightweight mode.", + "properties": execution_properties() + }) + }, + "delivery": { + "type": "object", + "description": "Optional delivery defaults for notifications and message tool calls inside routine jobs.", + "properties": delivery_properties() + }, + "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"] } + ]), + ); + schema_obj.insert( + "examples".to_string(), + Value::Array(routine_create_examples()), + ); + } + } 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) +} + +fn routine_create_discovery_schema() -> Value { + routine_create_schema(true) } -pub(crate) fn routine_update_parameters_schema() -> serde_json::Value { +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"]), + } +} + +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 +1056,24 @@ impl Tool for RoutineCreateTool { routine_create_parameters_schema() } + fn discovery_schema(&self) -> serde_json::Value { + routine_create_discovery_schema() + } + + fn discovery_summary(&self) -> Option { + Some(routine_create_tool_summary()) + } + 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 +1088,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: normalized.delivery.user.clone(), ..NotifyConfig::default() }, last_run_at: None, @@ -522,9 +1236,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 +1629,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 +1642,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 +1666,569 @@ 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}")) + } + + fn variant_with_kind<'a>(variants: &'a [Value], kind: &str) -> &'a Value { + variants + .iter() + .find(|variant| { + variant + .get("properties") + .and_then(Value::as_object) + .and_then(|properties| properties.get("kind")) + .and_then(|kind_schema| kind_schema.get("enum")) + .and_then(Value::as_array) + .is_some_and(|enums| enums.contains(&Value::String(kind.to_string()))) + }) + .unwrap_or_else(|| panic!("missing variant for kind={kind}")) + } + + fn variant_with_mode<'a>(variants: &'a [Value], mode: &str) -> &'a Value { + variants + .iter() + .find(|variant| { + variant + .get("properties") + .and_then(Value::as_object) + .and_then(|properties| properties.get("mode")) + .and_then(|mode_schema| mode_schema.get("enum")) + .and_then(Value::as_array) + .is_some_and(|enums| enums.contains(&Value::String(mode.to_string()))) + }) + .unwrap_or_else(|| panic!("missing variant for mode={mode}")) + } + + #[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!( + parsed.delivery.user.is_none(), + "expected omitted delivery.user to remain unspecified", + ); + } + + #[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_deref(), Some("ops-team")); + assert_eq!(parsed.cooldown_secs, 30); + } + + #[test] + 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 routine_create_schema_exposes_all_trigger_and_delivery_fields() { + 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_deref(), Some("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_deref(), Some("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_discovery_schema_splits_request_variants() { + let schema = routine_create_discovery_schema(); + let request = schema_property(&schema, "request"); + let variants = request + .get("oneOf") + .and_then(Value::as_array) + .expect("request.oneOf variants"); + assert_eq!(variants.len(), 4usize); + + let cron = variant_with_kind(variants, "cron"); + let cron_required = cron + .get("required") + .and_then(Value::as_array) + .expect("cron required list"); + assert!( + cron_required.contains(&Value::String("schedule".to_string())), + "cron variant should require schedule", + ); + + let message_event = variant_with_kind(variants, "message_event"); + let message_required = message_event + .get("required") + .and_then(Value::as_array) + .expect("message_event required list"); + assert!( + message_required.contains(&Value::String("pattern".to_string())), + "message_event variant should require pattern", + ); + + let system_event = variant_with_kind(variants, "system_event"); + let system_required = system_event + .get("required") + .and_then(Value::as_array) + .expect("system_event required list"); + assert!( + system_required.contains(&Value::String("source".to_string())) + && system_required.contains(&Value::String("event_type".to_string())), + "system_event variant should require source and event_type", + ); + } + + #[test] + fn routine_create_discovery_schema_splits_execution_variants() { + let schema = routine_create_discovery_schema(); + let execution = schema_property(&schema, "execution"); + let variants = execution + .get("oneOf") + .and_then(Value::as_array) + .expect("execution.oneOf variants"); + assert_eq!(variants.len(), 2usize); + + let lightweight = variant_with_mode(variants, "lightweight"); + let lightweight_props = lightweight + .get("properties") + .and_then(Value::as_object) + .expect("lightweight properties"); + assert!( + lightweight_props.contains_key("use_tools") + && lightweight_props.contains_key("context_paths") + && lightweight_props.contains_key("max_tool_rounds"), + "lightweight variant should expose lightweight-only fields", + ); + + let full_job = variant_with_mode(variants, "full_job"); + let full_job_props = full_job + .get("properties") + .and_then(Value::as_object) + .expect("full_job properties"); + assert!( + full_job_props.contains_key("tool_permissions"), + "full_job variant should expose tool_permissions", + ); + } + + #[test] + fn routine_create_discovery_summary_explains_rules_and_examples() { + let summary = routine_create_tool_summary(); + + assert_eq!( + summary.always_required, + vec![ + "name".to_string(), + "prompt".to_string(), + "request.kind".to_string() + ], + ); + assert!( + summary + .conditional_requirements + .iter() + .any(|rule| rule.contains("request.kind='cron'")), + "summary should explain cron requirement", + ); + assert!( + summary + .notes + .iter() + .any(|note| note.contains("Legacy flat aliases")), + "summary should mention legacy aliases", + ); + assert_eq!(summary.examples.len(), 4usize); + } + + #[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 +2237,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 +2248,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/builtin/tool_info.rs b/src/tools/builtin/tool_info.rs index cd94384d55..264547aae6 100644 --- a/src/tools/builtin/tool_info.rs +++ b/src/tools/builtin/tool_info.rs @@ -1,8 +1,9 @@ //! On-demand tool discovery (like CLI `--help`). //! -//! Two levels of detail: +//! Three levels of detail: //! - Default: name, description, parameter names (compact ~150 bytes) -//! - `include_schema: true`: adds the full typed JSON Schema +//! - `detail: "summary"`: adds curated rules, notes, and examples +//! - `detail: "schema"` / `include_schema: true`: adds the full typed JSON Schema //! //! Keeps the tools array compact (WASM tools use permissive schemas) //! while allowing precise discovery when needed. @@ -13,7 +14,59 @@ use async_trait::async_trait; use crate::context::JobContext; use crate::tools::registry::ToolRegistry; -use crate::tools::tool::{Tool, ToolError, ToolOutput, require_str}; +use crate::tools::tool::{Tool, ToolDiscoverySummary, ToolError, ToolOutput, require_str}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ToolInfoDetail { + Names, + Summary, + Schema, +} + +impl ToolInfoDetail { + fn parse(params: &serde_json::Value) -> Result { + if params + .get("include_schema") + .and_then(|v| v.as_bool()) + .unwrap_or(false) + { + return Ok(Self::Schema); + } + + match params.get("detail").and_then(|v| v.as_str()) { + None | Some("names") => Ok(Self::Names), + Some("summary") => Ok(Self::Summary), + Some("schema") => Ok(Self::Schema), + Some(other) => Err(ToolError::InvalidParameters(format!( + "invalid detail '{other}' (expected 'names', 'summary', or 'schema')" + ))), + } + } +} + +fn schema_param_names(schema: &serde_json::Value) -> Vec { + schema + .get("properties") + .and_then(|p| p.as_object()) + .map(|props| props.keys().cloned().collect()) + .unwrap_or_default() +} + +fn fallback_summary(schema: &serde_json::Value) -> ToolDiscoverySummary { + ToolDiscoverySummary { + always_required: schema + .get("required") + .and_then(|v| v.as_array()) + .map(|required| { + required + .iter() + .filter_map(|value| value.as_str().map(str::to_string)) + .collect() + }) + .unwrap_or_default(), + ..ToolDiscoverySummary::default() + } +} pub struct ToolInfoTool { registry: Weak, @@ -32,8 +85,7 @@ impl Tool for ToolInfoTool { } fn description(&self) -> &str { - "Get info about any tool: description and parameter names. \ - Set include_schema to true for the full typed parameter schema." + "Get info about any tool: description, parameter names, curated summary guidance, or full discovery schema." } fn parameters_schema(&self) -> serde_json::Value { @@ -44,9 +96,15 @@ impl Tool for ToolInfoTool { "type": "string", "description": "Name of the tool to get info about" }, + "detail": { + "type": "string", + "enum": ["names", "summary", "schema"], + "description": "Response detail level. 'names' returns parameter names only. 'summary' adds curated rules/examples. 'schema' returns the full discovery schema.", + "default": "names" + }, "include_schema": { "type": "boolean", - "description": "If true, include the full typed JSON Schema for parameters (larger response). Default: false.", + "description": "Deprecated compatibility alias for detail='schema'. If true, include the full discovery schema.", "default": false } }, @@ -61,10 +119,7 @@ impl Tool for ToolInfoTool { ) -> Result { let start = std::time::Instant::now(); let name = require_str(¶ms, "name")?; - let include_schema = params - .get("include_schema") - .and_then(|v| v.as_bool()) - .unwrap_or(false); + let detail = ToolInfoDetail::parse(¶ms)?; let registry = self.registry.upgrade().ok_or_else(|| { ToolError::ExecutionFailed( @@ -77,13 +132,7 @@ impl Tool for ToolInfoTool { })?; let schema = tool.discovery_schema(); - - // Extract just param names from the schema's "properties" keys - let param_names: Vec<&str> = schema - .get("properties") - .and_then(|p| p.as_object()) - .map(|props| props.keys().map(|k| k.as_str()).collect()) - .unwrap_or_default(); + let param_names = schema_param_names(&schema); let mut info = serde_json::json!({ "name": tool.name(), @@ -91,8 +140,21 @@ impl Tool for ToolInfoTool { "parameters": param_names, }); - if include_schema { - info["schema"] = schema; + match detail { + ToolInfoDetail::Names => {} + ToolInfoDetail::Summary => { + let summary = tool + .discovery_summary() + .unwrap_or_else(|| fallback_summary(&schema)); + info["summary"] = serde_json::to_value(summary).map_err(|err| { + ToolError::ExecutionFailed(format!( + "failed to serialize discovery summary: {err}" + )) + })?; + } + ToolInfoDetail::Schema => { + info["schema"] = schema; + } } Ok(ToolOutput::success(info, start.elapsed())) @@ -135,6 +197,30 @@ mod tests { assert!(info.get("schema").is_none()); } + #[tokio::test] + async fn test_tool_info_with_summary() { + let registry = Arc::new(ToolRegistry::new()); + registry.register(Arc::new(EchoTool)).await; + + let tool = ToolInfoTool::new(Arc::downgrade(®istry)); + let ctx = JobContext::default(); + let result = tool + .execute( + serde_json::json!({"name": "echo", "detail": "summary"}), + &ctx, + ) + .await + .unwrap(); + + let info = &result.result; + assert_eq!(info["name"], "echo"); + assert!(info["summary"].is_object()); + assert_eq!( + info["summary"]["always_required"], + serde_json::json!(["message"]) + ); + } + #[tokio::test] async fn test_tool_info_with_schema() { let registry = Arc::new(ToolRegistry::new()); @@ -157,6 +243,22 @@ mod tests { assert!(info["schema"]["properties"].is_object()); } + #[tokio::test] + async fn test_tool_info_invalid_detail() { + let registry = Arc::new(ToolRegistry::new()); + registry.register(Arc::new(EchoTool)).await; + + let tool = ToolInfoTool::new(Arc::downgrade(®istry)); + let ctx = JobContext::default(); + let result = tool + .execute( + serde_json::json!({"name": "echo", "detail": "verbose"}), + &ctx, + ) + .await; + assert!(matches!(result, Err(ToolError::InvalidParameters(_)))); + } + #[tokio::test] async fn test_tool_info_unknown_tool() { let registry = Arc::new(ToolRegistry::new()); diff --git a/src/tools/registry.rs b/src/tools/registry.rs index 0c457a6d3c..f8110b4657 100644 --- a/src/tools/registry.rs +++ b/src/tools/registry.rs @@ -94,6 +94,15 @@ pub struct ToolRegistry { } impl ToolRegistry { + fn tool_definition(tool: &Arc) -> ToolDefinition { + let schema = tool.schema(); + ToolDefinition { + name: schema.name, + description: schema.description, + parameters: schema.parameters, + } + } + /// Create a new empty registry. pub fn new() -> Self { Self { @@ -206,11 +215,7 @@ impl ToolRegistry { .read() .await .values() - .map(|tool| ToolDefinition { - name: tool.name().to_string(), - description: tool.description().to_string(), - parameters: tool.parameters_schema(), - }) + .map(Self::tool_definition) .collect(); defs.sort_unstable_by(|a, b| a.name.cmp(&b.name)); defs @@ -221,13 +226,7 @@ impl ToolRegistry { let tools = self.tools.read().await; names .iter() - .filter_map(|name| { - tools.get(*name).map(|tool| ToolDefinition { - name: tool.name().to_string(), - description: tool.description().to_string(), - parameters: tool.parameters_schema(), - }) - }) + .filter_map(|name| tools.get(*name).map(Self::tool_definition)) .collect() } @@ -282,11 +281,7 @@ impl ToolRegistry { .await .values() .filter(|tool| tool.domain() == domain) - .map(|tool| ToolDefinition { - name: tool.name().to_string(), - description: tool.description().to_string(), - parameters: tool.parameters_schema(), - }) + .map(Self::tool_definition) .collect() } @@ -312,11 +307,7 @@ impl ToolRegistry { ApprovalRequirement::Never ) }) - .map(|tool| ToolDefinition { - name: tool.name().to_string(), - description: tool.description().to_string(), - parameters: tool.parameters_schema(), - }) + .map(Self::tool_definition) .collect(); defs.sort_unstable_by(|a, b| a.name.cmp(&b.name)); defs @@ -788,6 +779,7 @@ impl std::fmt::Debug for ToolRegistry { mod tests { use super::*; use crate::tools::registry::EchoTool; + use crate::tools::tool::ToolDiscoverySummary; #[tokio::test] async fn test_register_and_get() { @@ -818,6 +810,71 @@ mod tests { assert_eq!(defs[0].name, "echo"); } + #[tokio::test] + async fn test_tool_definitions_use_tool_schema() { + struct DiscoveryTool; + + #[async_trait::async_trait] + impl Tool for DiscoveryTool { + fn name(&self) -> &str { + "discovery_tool" + } + + fn description(&self) -> &str { + "Discovery test tool" + } + + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "name": { "type": "string" } + } + }) + } + + fn discovery_schema(&self) -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "name": { "type": "string" }, + "extra": { "type": "string" } + } + }) + } + + fn discovery_summary(&self) -> Option { + Some(ToolDiscoverySummary { + notes: vec!["extra guidance".into()], + ..ToolDiscoverySummary::default() + }) + } + + async fn execute( + &self, + _params: serde_json::Value, + _ctx: &crate::context::JobContext, + ) -> Result { + unreachable!() + } + } + + let registry = ToolRegistry::new(); + registry.register(Arc::new(DiscoveryTool)).await; + + let defs = registry.tool_definitions().await; + let def = defs + .iter() + .find(|def| def.name == "discovery_tool") + .expect("tool definition should be present"); + assert!( + def.description.contains("tool_info"), + "live tool definition should include schema hint: {}", + def.description + ); + assert!(def.parameters.get("extra").is_none()); + } + #[tokio::test] async fn test_builtin_tool_cannot_be_shadowed() { let registry = ToolRegistry::new(); diff --git a/src/tools/schema_validator.rs b/src/tools/schema_validator.rs index 9cc2fa5f3c..df87afa4e8 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/src/tools/tool.rs b/src/tools/tool.rs index 608c71a65c..e80712a93d 100644 --- a/src/tools/tool.rs +++ b/src/tools/tool.rs @@ -231,6 +231,19 @@ impl ToolSchema { } } +/// Curated discovery guidance surfaced by `tool_info(detail: "summary")`. +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +pub struct ToolDiscoverySummary { + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub always_required: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub conditional_requirements: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub notes: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub examples: Vec, +} + /// Trait for tools that the agent can use. #[async_trait] pub trait Tool: Send + Sync { @@ -347,12 +360,32 @@ pub trait Tool: Send + Sync { self.parameters_schema() } + /// Curated discovery guidance used by `tool_info(detail: "summary")`. + /// + /// Default: no custom summary; callers may derive a minimal fallback from + /// `discovery_schema()`. + fn discovery_summary(&self) -> Option { + None + } + /// Get the tool schema for LLM function calling. fn schema(&self) -> ToolSchema { + let parameters = self.parameters_schema(); + let has_discovery_hint = + self.discovery_summary().is_some() || self.discovery_schema() != parameters; + let description = if has_discovery_hint { + format!( + "{} (call tool_info(name: \"{}\", detail: \"summary\") for rules/examples or detail: \"schema\" for the full discovery schema)", + self.description(), + self.name() + ) + } else { + self.description().to_string() + }; ToolSchema { name: self.name().to_string(), - description: self.description().to_string(), - parameters: self.parameters_schema(), + description, + parameters, } } } diff --git a/tests/e2e_builtin_tool_coverage.rs b/tests/e2e_builtin_tool_coverage.rs index 2a97a0d503..d08f220431 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.as_deref(), Some("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] @@ -571,10 +698,11 @@ mod tests { } // ----------------------------------------------------------------------- - // Test: tool_info_discovery (two-level detail) + // Test: tool_info_discovery (three-level detail) // ----------------------------------------------------------------------- // Verifies the tool_info built-in returns: // - Default (no include_schema): name, description, parameter names array + // - `detail: "summary"`: curated summary guidance // - With include_schema: true: adds full typed JSON Schema #[tokio::test] @@ -597,13 +725,13 @@ mod tests { rig.verify_trace_expects(&trace, &responses); - // tool_info should have been called twice (echo + time), both succeeding. + // tool_info should have been called three times (echo + routine_create + time), all succeeding. let completed = rig.tool_calls_completed(); let tool_info_calls: Vec<_> = completed.iter().filter(|(n, _)| n == "tool_info").collect(); assert_eq!( tool_info_calls.len(), - 2, - "Expected 2 tool_info calls, got {tool_info_calls:?}" + 3, + "Expected 3 tool_info calls, got {tool_info_calls:?}" ); assert!( tool_info_calls.iter().all(|(_, ok)| *ok), @@ -613,44 +741,71 @@ mod tests { // Verify the results contain expected fields. let results = rig.tool_results(); let info_results: Vec<_> = results.iter().filter(|(n, _)| n == "tool_info").collect(); + let info_json: Vec = info_results + .iter() + .map(|(_, preview)| { + serde_json::from_str(preview) + .expect("tool_info result preview should be valid JSON") + }) + .collect(); // First call was for "echo" (default, no include_schema) — result should // contain "echo" and "parameters" as an array of names (not full schema). - let echo_result = info_results + let echo_json = info_json .iter() - .find(|(_, preview)| preview.contains("echo")) + .find(|info| info["name"] == "echo") .expect("tool_info result should contain 'echo'"); assert!( - echo_result.1.contains("message"), + echo_json["parameters"] + .as_array() + .is_some_and(|params| params.iter().any(|param| param == "message")), "echo default result should list 'message' parameter name: {:?}", - echo_result.1 + echo_json ); // Default mode should NOT include the full "schema" key - let echo_json: serde_json::Value = serde_json::from_str(&echo_result.1) - .expect("echo tool_info result should be valid JSON"); assert!( echo_json.get("schema").is_none(), "Default tool_info should not include schema field: {:?}", - echo_result.1 + echo_json + ); + + // Second call was for "routine_create" with detail: "summary" — result + // should contain a summary object with rules/examples. + let routine_json = info_json + .iter() + .find(|info| info["name"] == "routine_create") + .expect("tool_info result should contain 'routine_create'"); + assert!( + routine_json.get("summary").is_some(), + "detail: summary should include summary field: {:?}", + routine_json + ); + assert!( + routine_json["summary"]["conditional_requirements"] + .as_array() + .is_some_and(|rules| rules.iter().any(|rule| { + rule.as_str() + .is_some_and(|rule| rule.contains("request.kind='cron'")) + })), + "routine_create summary should mention cron requirement: {:?}", + routine_json ); - // Second call was for "time" with include_schema: true — result should + // Third call was for "time" with include_schema: true — result should // contain "time", "schema" field with full object. - let time_result = info_results + let time_json = info_json .iter() - .find(|(_, preview)| preview.contains("time")) + .find(|info| info["name"] == "time") .expect("tool_info result should contain 'time'"); - let time_json: serde_json::Value = serde_json::from_str(&time_result.1) - .expect("time tool_info result should be valid JSON"); assert!( time_json.get("schema").is_some(), "include_schema: true should include schema field: {:?}", - time_result.1 + time_json ); assert!( time_json["schema"]["properties"].is_object(), "schema should have properties: {:?}", - time_result.1 + time_json ); rig.shutdown(); 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 0000000000..ae4b6eb95a --- /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 0000000000..61f159c0ad --- /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 + } + } + ] +} diff --git a/tests/fixtures/llm_traces/tools/tool_info_discovery.json b/tests/fixtures/llm_traces/tools/tool_info_discovery.json index dc8746ad7c..5a18e9e966 100644 --- a/tests/fixtures/llm_traces/tools/tool_info_discovery.json +++ b/tests/fixtures/llm_traces/tools/tool_info_discovery.json @@ -24,6 +24,20 @@ "output_tokens": 20 } }, + { + "response": { + "type": "tool_calls", + "tool_calls": [ + { + "id": "call_tool_info_routine_create", + "name": "tool_info", + "arguments": { "name": "routine_create", "detail": "summary" } + } + ], + "input_tokens": 160, + "output_tokens": 25 + } + }, { "response": { "type": "tool_calls", @@ -34,16 +48,16 @@ "arguments": { "name": "time", "include_schema": true } } ], - "input_tokens": 200, + "input_tokens": 240, "output_tokens": 20 } }, { "response": { "type": "text", - "content": "I found the info for both tools. The echo tool has a 'message' parameter. The time tool accepts an 'operation' parameter with options like 'now', 'parse', and 'diff'.", - "input_tokens": 400, - "output_tokens": 40 + "content": "I found the info for all three tools. The echo tool has a 'message' parameter. routine_create's summary explains that cron needs request.schedule, message_event needs request.pattern, and system_event needs request.source plus request.event_type. The time tool accepts an 'operation' parameter with options like 'now', 'parse', and 'diff'.", + "input_tokens": 520, + "output_tokens": 60 } } ]