diff --git a/crates/goose-cli/src/session/output.rs b/crates/goose-cli/src/session/output.rs index 0e214d077dbd..2c46408ed031 100644 --- a/crates/goose-cli/src/session/output.rs +++ b/crates/goose-cli/src/session/output.rs @@ -413,13 +413,8 @@ fn render_text_editor_request(call: &ToolCall, debug: bool) { fn render_shell_request(call: &ToolCall, debug: bool) { print_tool_header(call); - - match call.arguments.get("command") { - Some(Value::String(s)) => { - println!("{}: {}", style("command").dim(), style(s).green()); - } - _ => print_params(&call.arguments, 0, debug), - } + print_params(&call.arguments, 0, debug); + println!(); } fn render_dynamic_task_request(call: &ToolCall, debug: bool) { @@ -439,10 +434,36 @@ fn render_dynamic_task_request(call: &ToolCall, debug: bool) { // For strings, print the full content without truncation println!(" {}: {}", style(key).dim(), style(s).green()); } + Value::Array(arr) => { + // For arrays, print each item on its own line + println!(" {}:", style(key).dim()); + for item in arr { + if let Value::String(s) = item { + println!(" - {}", style(s).green()); + } else if let Value::Object(_) = item { + // For objects in arrays, print them with indentation + print!(" - "); + print_params(item, 3, debug); + } else { + println!( + " - {}", + style(format!("{}", item)).green() + ); + } + } + } + Value::Object(_) => { + // For objects, print them with proper indentation + println!(" {}:", style(key).dim()); + print_params(value, 2, debug); + } _ => { - // For everything else, use print_params - print!(" "); - print_params(value, 0, debug); + // For other types (numbers, booleans, null) + println!( + " {}: {}", + style(key).dim(), + style(format!("{}", value)).green() + ); } } } diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index 01c417984160..f16240c30bc4 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -454,7 +454,18 @@ impl Agent { ) .await } else if tool_call.name == DYNAMIC_TASK_TOOL_NAME_PREFIX { - create_dynamic_task(tool_call.arguments.clone(), &self.tasks_manager).await + // Get loaded extensions for shortname resolution + let loaded_extensions = self + .extension_manager + .list_extensions() + .await + .unwrap_or_default(); + create_dynamic_task( + tool_call.arguments.clone(), + &self.tasks_manager, + loaded_extensions, + ) + .await } else if tool_call.name == PLATFORM_READ_RESOURCE_TOOL_NAME { // Check if the tool is read_resource and handle it separately ToolCallResult::from( diff --git a/crates/goose/src/agents/mod.rs b/crates/goose/src/agents/mod.rs index c18cfd916075..c5f7a5e485de 100644 --- a/crates/goose/src/agents/mod.rs +++ b/crates/goose/src/agents/mod.rs @@ -6,7 +6,7 @@ pub mod final_output_tool; mod large_response_handler; pub mod platform_tools; pub mod prompt_manager; -mod recipe_tools; +pub mod recipe_tools; mod reply_parts; pub mod retry; mod router_tool_selector; diff --git a/crates/goose/src/agents/recipe_tools/dynamic_task_tools.rs b/crates/goose/src/agents/recipe_tools/dynamic_task_tools.rs index 823235087da8..4386364559a1 100644 --- a/crates/goose/src/agents/recipe_tools/dynamic_task_tools.rs +++ b/crates/goose/src/agents/recipe_tools/dynamic_task_tools.rs @@ -2,9 +2,15 @@ // Module: Dynamic Task Tools // Handles creation of tasks dynamically without sub-recipes // ======================================= +use crate::agents::extension::ExtensionConfig; use crate::agents::subagent_execution_tool::tasks_manager::TasksManager; -use crate::agents::subagent_execution_tool::{lib::ExecutionMode, task_types::Task}; +use crate::agents::subagent_execution_tool::{ + lib::ExecutionMode, + task_types::{Task, TaskType}, +}; use crate::agents::tool_execution::ToolCallResult; +use crate::recipe::{Recipe, RecipeBuilder}; +use anyhow::{anyhow, Result}; use rmcp::model::{Content, ErrorCode, ErrorData, Tool, ToolAnnotations}; use rmcp::object; use serde_json::{json, Value}; @@ -15,58 +21,207 @@ pub const DYNAMIC_TASK_TOOL_NAME_PREFIX: &str = "dynamic_task__create_task"; pub fn create_dynamic_task_tool() -> Tool { Tool::new( DYNAMIC_TASK_TOOL_NAME_PREFIX.to_string(), - "Use this tool to create one or more dynamic tasks from a shared text instruction and varying parameters.\ - How it works: - - Provide a single text instruction - - Use the 'task_parameters' field to pass an array of parameter sets - - Each resulting task will use the same instruction with different parameter values - This is useful when performing the same operation across many inputs (e.g., getting weather for multiple cities, searching multiple slack channels, iterating through various linear tickets, etc). - Once created, these tasks should be passed to the 'subagent__execute_task' tool for execution. Tasks can run sequentially or in parallel. - --- - What is a 'subagent'? - A 'subagent' is a stateless sub-process that executes a single task independently. Use subagents when: - - You want to parallelize similar work across different inputs - - You are not sure your search or operation will succeed on the first try - Each subagent receives a task with a defined payload and returns a result, which is not visible to the user unless explicitly summarized by the system. - --- - Examples of 'task_parameters' for a single task: - text_instruction: Search for the config file in the root directory. - Examples of 'task_parameters' for multiple tasks: - text_instruction: Get weather for Melbourne. - text_instruction: Get weather for Los Angeles. - text_instruction: Get weather for San Francisco. - ".to_string(), + "Create tasks with instructions or prompt. For simple tasks, only include the instructions field. Extensions control: omit field = use all current extensions; empty array [] = no extensions; array with names = only those extensions. Specify extensions as shortnames (the prefixes for your tools). Specify return_last_only as true and have your subagent summarize its work in its last message to conserve your own context. Optional: title, description, extensions, settings, retry, response schema, context, activities. Arrays for multiple tasks.".to_string(), object!({ "type": "object", "properties": { "task_parameters": { "type": "array", - "description": "Array of parameter sets for creating tasks. \ - For a single task, provide an array with one element. \ - For multiple tasks, provide an array with multiple elements, each with different parameter values. \ - If there is no parameter set, provide an empty array.", + "description": "Array of tasks. Each needs 'instructions' OR 'prompt'.", "items": { "type": "object", "properties": { - "text_instruction": { + // Required (one of these) + "instructions": { "type": "string", - "description": "The text instruction to execute" + "description": "Task instructions" }, + "prompt": { + "type": "string", + "description": "Initial prompt" + }, + // Optional - auto-generated if not provided + "title": {"type": "string"}, + "description": {"type": "string"}, + "extensions": {"type": "array"}, + "settings": {"type": "object"}, + "parameters": {"type": "array"}, + "response": {"type": "object"}, + "retry": {"type": "object"}, + "context": {"type": "array"}, + "activities": {"type": "array"}, + "return_last_only": { + "type": "boolean", + "description": "If true, return only the last message from the subagent (default: false, returns full conversation)" + } }, - "required": ["text_instruction"] - } + "anyOf": [ + {"required": ["instructions"]}, + {"required": ["prompt"]} + ] + }, + "minItems": 1 + }, + "execution_mode": { + "type": "string", + "enum": ["sequential", "parallel"], + "description": "How to execute multiple tasks (default: parallel for multiple tasks, sequential for single task)" } - } + }, + "required": ["task_parameters"] }) ).annotate(ToolAnnotations { - title: Some("Dynamic Task Creation".to_string()), + title: Some("Create Dynamic Tasks".to_string()), read_only_hint: Some(false), - destructive_hint: Some(true), + destructive_hint: Some(false), idempotent_hint: Some(false), open_world_hint: Some(true), }) } +fn process_extensions( + extensions: &Value, + _loaded_extensions: &[String], +) -> Option> { + // First try to deserialize as ExtensionConfig array + if let Ok(ext_configs) = serde_json::from_value::>(extensions.clone()) { + return Some(ext_configs); + } + + // Try to handle mixed array of strings and objects + if let Some(arr) = extensions.as_array() { + // If the array is empty, return an empty Vec (not None) + // This is important: empty array means "no extensions" + if arr.is_empty() { + return Some(Vec::new()); + } + + let mut converted_extensions = Vec::new(); + + for ext in arr { + if let Some(name_str) = ext.as_str() { + // Look up the full extension config by name + match crate::config::ExtensionConfigManager::get_config_by_name(name_str) { + Ok(Some(config)) => { + // Check if the extension is enabled + if crate::config::ExtensionConfigManager::is_enabled(&config.key()) + .unwrap_or(false) + { + converted_extensions.push(config); + } else { + tracing::warn!("Extension '{}' is disabled, skipping", name_str); + } + } + Ok(None) => { + tracing::warn!("Extension '{}' not found in configuration", name_str); + } + Err(e) => { + tracing::warn!("Error looking up extension '{}': {}", name_str, e); + } + } + } else if let Ok(ext_config) = serde_json::from_value::(ext.clone()) { + converted_extensions.push(ext_config); + } + } + + // Return the converted extensions even if empty + // (empty means user explicitly wants no extensions) + return Some(converted_extensions); + } + None +} + +// Helper function to apply recipe builder methods if value can be deserialized +fn apply_if_ok( + builder: RecipeBuilder, + value: Option<&Value>, + f: impl FnOnce(RecipeBuilder, T) -> RecipeBuilder, +) -> RecipeBuilder { + match value.and_then(|v| serde_json::from_value(v.clone()).ok()) { + Some(parsed) => f(builder, parsed), + None => builder, + } +} + +pub fn task_params_to_inline_recipe( + task_param: &Value, + loaded_extensions: &[String], +) -> Result { + // Extract and validate core fields + let instructions = task_param.get("instructions").and_then(|v| v.as_str()); + let prompt = task_param.get("prompt").and_then(|v| v.as_str()); + + if instructions.is_none() && prompt.is_none() { + return Err(anyhow!("Either 'instructions' or 'prompt' is required")); + } + + // Build recipe with auto-generated defaults + let mut builder = Recipe::builder() + .version("1.0.0") + .title( + task_param + .get("title") + .and_then(|v| v.as_str()) + .unwrap_or("Dynamic Task"), + ) + .description( + task_param + .get("description") + .and_then(|v| v.as_str()) + .unwrap_or("Inline recipe task"), + ); + + // Set instructions/prompt + if let Some(inst) = instructions { + builder = builder.instructions(inst); + } + if let Some(p) = prompt { + builder = builder.prompt(p); + } + + // Handle extensions + if let Some(extensions) = task_param.get("extensions") { + if let Some(ext_configs) = process_extensions(extensions, loaded_extensions) { + builder = builder.extensions(ext_configs); + } + } + + // Handle other optional fields + builder = apply_if_ok(builder, task_param.get("settings"), RecipeBuilder::settings); + builder = apply_if_ok(builder, task_param.get("response"), RecipeBuilder::response); + builder = apply_if_ok(builder, task_param.get("retry"), RecipeBuilder::retry); + builder = apply_if_ok(builder, task_param.get("context"), RecipeBuilder::context); + builder = apply_if_ok( + builder, + task_param.get("activities"), + RecipeBuilder::activities, + ); + builder = apply_if_ok( + builder, + task_param.get("parameters"), + RecipeBuilder::parameters, + ); + + // Build and validate + let recipe = builder + .build() + .map_err(|e| anyhow!("Failed to build recipe: {}", e))?; + + // Security validation + if recipe.check_for_security_warnings() { + return Err(anyhow!("Recipe contains potentially harmful content")); + } + + // Validate retry config if present + if let Some(ref retry) = recipe.retry { + retry + .validate() + .map_err(|e| anyhow!("Invalid retry config: {}", e))?; + } + + Ok(recipe) +} + fn extract_task_parameters(params: &Value) -> Vec { params .get("task_parameters") @@ -75,29 +230,6 @@ fn extract_task_parameters(params: &Value) -> Vec { .unwrap_or_default() } -fn create_text_instruction_tasks_from_params(task_params: &[Value]) -> Vec { - task_params - .iter() - .map(|task_param| { - let text_instruction = task_param - .get("text_instruction") - .and_then(|v| v.as_str()) - .unwrap_or("") - .to_string(); - - let payload = json!({ - "text_instruction": text_instruction - }); - - Task { - id: uuid::Uuid::new_v4().to_string(), - task_type: "text_instruction".to_string(), - payload, - } - }) - .collect() -} - fn create_task_execution_payload(tasks: Vec, execution_mode: ExecutionMode) -> Value { let task_ids: Vec = tasks.iter().map(|task| task.id.clone()).collect(); json!({ @@ -106,25 +238,79 @@ fn create_task_execution_payload(tasks: Vec, execution_mode: ExecutionMode }) } -pub async fn create_dynamic_task(params: Value, tasks_manager: &TasksManager) -> ToolCallResult { +pub async fn create_dynamic_task( + params: Value, + tasks_manager: &TasksManager, + loaded_extensions: Vec, +) -> ToolCallResult { let task_params_array = extract_task_parameters(¶ms); if task_params_array.is_empty() { return ToolCallResult::from(Err(ErrorData { - code: ErrorCode::INTERNAL_ERROR, - message: Cow::from("No task parameters provided"), + code: ErrorCode::INVALID_PARAMS, + message: Cow::from("task_parameters array cannot be empty"), data: None, })); } - let tasks = create_text_instruction_tasks_from_params(&task_params_array); + // Convert each parameter set to inline recipe and create tasks + let mut tasks = Vec::new(); + for task_param in &task_params_array { + // All tasks must use the new inline recipe path + match task_params_to_inline_recipe(task_param, &loaded_extensions) { + Ok(recipe) => { + let recipe_json = match serde_json::to_value(&recipe) { + Ok(json) => json, + Err(e) => { + return ToolCallResult::from(Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::from(format!("Failed to serialize recipe: {}", e)), + data: None, + })); + } + }; - // Use parallel execution if there are multiple tasks, sequential for single task - let execution_mode = if tasks.len() > 1 { - ExecutionMode::Parallel - } else { - ExecutionMode::Sequential - }; + // Extract return_last_only flag if present + let return_last_only = task_param + .get("return_last_only") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + + let task = Task { + id: uuid::Uuid::new_v4().to_string(), + task_type: TaskType::InlineRecipe, + payload: json!({ + "recipe": recipe_json, + "return_last_only": return_last_only + }), + }; + tasks.push(task); + } + Err(e) => { + return ToolCallResult::from(Err(ErrorData { + code: ErrorCode::INVALID_PARAMS, + message: Cow::from(format!("Invalid task parameters: {}", e)), + data: None, + })); + } + } + } + + let execution_mode = params + .get("execution_mode") + .and_then(|v| v.as_str()) + .map(|s| match s { + "sequential" => ExecutionMode::Sequential, + "parallel" => ExecutionMode::Parallel, + _ => ExecutionMode::Parallel, + }) + .unwrap_or_else(|| { + if tasks.len() > 1 { + ExecutionMode::Parallel + } else { + ExecutionMode::Sequential + } + }); let task_execution_payload = create_task_execution_payload(tasks.clone(), execution_mode); @@ -138,6 +324,7 @@ pub async fn create_dynamic_task(params: Value, tasks_manager: &TasksManager) -> })) } }; - tasks_manager.save_tasks(tasks.clone()).await; + + tasks_manager.save_tasks(tasks).await; ToolCallResult::from(Ok(vec![Content::text(tasks_json)])) } diff --git a/crates/goose/src/agents/recipe_tools/sub_recipe_tools.rs b/crates/goose/src/agents/recipe_tools/sub_recipe_tools.rs index 66da9ca22d2e..8e93a658e933 100644 --- a/crates/goose/src/agents/recipe_tools/sub_recipe_tools.rs +++ b/crates/goose/src/agents/recipe_tools/sub_recipe_tools.rs @@ -6,7 +6,8 @@ use anyhow::Result; use rmcp::model::{Tool, ToolAnnotations}; use serde_json::{json, Map, Value}; -use crate::agents::subagent_execution_tool::lib::{ExecutionMode, Task}; +use crate::agents::subagent_execution_tool::lib::ExecutionMode; +use crate::agents::subagent_execution_tool::task_types::{Task, TaskType}; use crate::agents::subagent_execution_tool::tasks_manager::TasksManager; use crate::recipe::{Recipe, RecipeParameter, RecipeParameterRequirement, SubRecipe}; @@ -67,7 +68,7 @@ fn create_tasks_from_params( }); Task { id: uuid::Uuid::new_v4().to_string(), - task_type: "sub_recipe".to_string(), + task_type: TaskType::SubRecipe, payload, } }) diff --git a/crates/goose/src/agents/subagent.rs b/crates/goose/src/agents/subagent.rs index 50eded2b30fd..5f292345e1e9 100644 --- a/crates/goose/src/agents/subagent.rs +++ b/crates/goose/src/agents/subagent.rs @@ -59,20 +59,25 @@ impl SubAgent { // Create a new extension manager for this subagent let extension_manager = ExtensionManager::new(); - // Add extensions based on task_type: - // 1. If executing dynamic task (task_type = 'text_instruction'), default to using all enabled extensions - // 2. (TODO) If executing a sub-recipe task, only use recipe extensions - - // Get all enabled extensions from config - let enabled_extensions = ExtensionConfigManager::get_all() - .unwrap_or_default() - .into_iter() - .filter(|ext| ext.enabled) - .map(|ext| ext.config) - .collect::>(); - - // Add enabled extensions to the subagent's extension manager - for extension in enabled_extensions { + // Determine which extensions to add: + // 1. If task_config.extensions is Some(vec), use those specific extensions + // 2. If task_config.extensions is None, use all enabled extensions (backward compatibility) + + let extensions_to_add = if let Some(ref extensions) = task_config.extensions { + // Use the explicitly specified extensions + extensions.clone() + } else { + // Default behavior: use all enabled extensions + ExtensionConfigManager::get_all() + .unwrap_or_default() + .into_iter() + .filter(|ext| ext.enabled) + .map(|ext| ext.config) + .collect::>() + }; + + // Add the determined extensions to the subagent's extension manager + for extension in extensions_to_add { if let Err(e) = extension_manager.add_extension(extension).await { debug!("Failed to add extension to subagent: {}", e); // Continue with other extensions even if one fails diff --git a/crates/goose/src/agents/subagent_execution_tool/task_execution_tracker.rs b/crates/goose/src/agents/subagent_execution_tool/task_execution_tracker.rs index da8ea0ca121b..8833c21e2f48 100644 --- a/crates/goose/src/agents/subagent_execution_tool/task_execution_tracker.rs +++ b/crates/goose/src/agents/subagent_execution_tool/task_execution_tracker.rs @@ -44,8 +44,6 @@ fn format_task_metadata(task_info: &TaskInfo) -> String { }) .collect::>() .join(",") - } else if let Some(text_instruction) = task_info.task.get_text_instruction() { - format!("instruction={}", text_instruction) } else { String::new() } @@ -234,7 +232,7 @@ impl TaskExecutionTracker { } }), current_output: task_info.current_output.clone(), - task_type: task_info.task.task_type.clone(), + task_type: task_info.task.task_type.to_string(), task_name: get_task_name(task_info).to_string(), task_metadata: format_task_metadata(task_info), error: task_info.error().cloned(), diff --git a/crates/goose/src/agents/subagent_execution_tool/task_types.rs b/crates/goose/src/agents/subagent_execution_tool/task_types.rs index 6bdcce33a7f9..3defebd02301 100644 --- a/crates/goose/src/agents/subagent_execution_tool/task_types.rs +++ b/crates/goose/src/agents/subagent_execution_tool/task_types.rs @@ -1,5 +1,6 @@ use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; +use std::fmt; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; use tokio::sync::mpsc; @@ -15,16 +16,32 @@ pub enum ExecutionMode { Parallel, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "snake_case")] +pub enum TaskType { + InlineRecipe, + SubRecipe, +} + +impl fmt::Display for TaskType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + TaskType::InlineRecipe => write!(f, "inline_recipe"), + TaskType::SubRecipe => write!(f, "sub_recipe"), + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Task { pub id: String, - pub task_type: String, + pub task_type: TaskType, pub payload: Value, } impl Task { pub fn get_sub_recipe(&self) -> Option<&Map> { - (self.task_type == "sub_recipe") + matches!(self.task_type, TaskType::SubRecipe) .then(|| self.payload.get("sub_recipe")?.as_object()) .flatten() } @@ -52,16 +69,6 @@ impl Task { .and_then(|sr| sr.get("recipe_path")) .and_then(|path| path.as_str()) } - - pub fn get_text_instruction(&self) -> Option<&str> { - if self.task_type != "sub_recipe" { - self.payload - .get("text_instruction") - .and_then(|text| text.as_str()) - } else { - None - } - } } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/crates/goose/src/agents/subagent_execution_tool/tasks.rs b/crates/goose/src/agents/subagent_execution_tool/tasks.rs index a363f6d30cd3..96d9f25d0f33 100644 --- a/crates/goose/src/agents/subagent_execution_tool/tasks.rs +++ b/crates/goose/src/agents/subagent_execution_tool/tasks.rs @@ -6,9 +6,8 @@ use tokio::process::Command; use tokio_util::sync::CancellationToken; use crate::agents::subagent_execution_tool::task_execution_tracker::TaskExecutionTracker; -use crate::agents::subagent_execution_tool::task_types::{Task, TaskResult, TaskStatus}; +use crate::agents::subagent_execution_tool::task_types::{Task, TaskResult, TaskStatus, TaskType}; use crate::agents::subagent_execution_tool::utils::strip_ansi_codes; -use crate::agents::subagent_handler::run_complete_subagent_task; use crate::agents::subagent_task_config::TaskConfig; pub async fn process_task( @@ -46,60 +45,71 @@ async fn get_task_result( task_config: TaskConfig, cancellation_token: CancellationToken, ) -> Result { - if task.task_type == "text_instruction" { - // Handle text_instruction tasks using subagent system - handle_text_instruction_task( - task, - task_execution_tracker, - task_config, - cancellation_token, - ) - .await - } else { - // Handle sub_recipe tasks using command execution - let (command, output_identifier) = build_command(&task)?; - let (stdout_output, stderr_output, success) = run_command( - command, - &output_identifier, - &task.id, - task_execution_tracker, - cancellation_token, - ) - .await?; - - if success { - process_output(stdout_output) - } else { - Err(format!("Command failed:\n{}", &stderr_output)) + match task.task_type { + TaskType::InlineRecipe => { + handle_inline_recipe_task(task, task_config, cancellation_token).await + } + TaskType::SubRecipe => { + let (command, output_identifier) = build_command(&task)?; + let (stdout_output, stderr_output, success) = run_command( + command, + &output_identifier, + &task.id, + task_execution_tracker, + cancellation_token, + ) + .await?; + + if success { + process_output(stdout_output) + } else { + Err(format!("Command failed:\n{}", &stderr_output)) + } } } } -async fn handle_text_instruction_task( +async fn handle_inline_recipe_task( task: Task, - task_execution_tracker: Arc, - task_config: TaskConfig, + mut task_config: TaskConfig, cancellation_token: CancellationToken, ) -> Result { - let text_instruction = task - .get_text_instruction() - .ok_or_else(|| format!("Task {}: Missing text_instruction", task.id))?; + use crate::agents::subagent_handler::run_complete_subagent_task_with_options; + use crate::recipe::Recipe; + + let recipe_value = task + .payload + .get("recipe") + .ok_or_else(|| "Missing recipe in inline_recipe task payload".to_string())?; + + let recipe: Recipe = serde_json::from_value(recipe_value.clone()) + .map_err(|e| format!("Invalid recipe in payload: {}", e))?; - // Start tracking the task - task_execution_tracker.start_task(&task.id).await; + let return_last_only = task + .payload + .get("return_last_only") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + task_config.extensions = recipe.extensions.clone(); + + let instruction = recipe + .instructions + .or(recipe.prompt) + .ok_or_else(|| "No instructions or prompt in recipe".to_string())?; let result = tokio::select! { - result = run_complete_subagent_task(text_instruction.to_string(), task_config) => result, + result = run_complete_subagent_task_with_options(instruction, task_config, return_last_only) => result, _ = cancellation_token.cancelled() => { return Err("Task cancelled".to_string()); } }; + match result { Ok(result_text) => Ok(serde_json::json!({ "result": result_text })), Err(e) => { - let error_msg = format!("Subagent execution failed: {}", e); + let error_msg = format!("Inline recipe execution failed: {}", e); Err(error_msg) } } @@ -108,36 +118,39 @@ async fn handle_text_instruction_task( fn build_command(task: &Task) -> Result<(Command, String), String> { let task_error = |field: &str| format!("Task {}: Missing {}", task.id, field); - let (mut command, output_identifier) = if task.task_type == "sub_recipe" { - let sub_recipe_name = task - .get_sub_recipe_name() - .ok_or_else(|| task_error("sub_recipe name"))?; - let path = task - .get_sub_recipe_path() - .ok_or_else(|| task_error("sub_recipe path"))?; - let command_parameters = task - .get_command_parameters() - .ok_or_else(|| task_error("command_parameters"))?; - - let mut cmd = Command::new("goose"); - cmd.arg("run").arg("--recipe").arg(path).arg("--no-session"); - - for (key, value) in command_parameters { - let key_str = key.to_string(); - let value_str = value.as_str().unwrap_or(&value.to_string()).to_string(); - cmd.arg("--params") - .arg(format!("{}={}", key_str, value_str)); - } - (cmd, format!("sub-recipe {}", sub_recipe_name)) - } else { - // This branch should not be reached for text_instruction tasks anymore - // as they are handled in handle_text_instruction_task - return Err("Text instruction tasks are handled separately".to_string()); - }; + if !matches!(task.task_type, TaskType::SubRecipe) { + return Err("Only sub-recipe tasks can be executed as commands".to_string()); + } + + let sub_recipe_name = task + .get_sub_recipe_name() + .ok_or_else(|| task_error("sub_recipe name"))?; + let path = task + .get_sub_recipe_path() + .ok_or_else(|| task_error("sub_recipe path"))?; + let command_parameters = task + .get_command_parameters() + .ok_or_else(|| task_error("command_parameters"))?; + + let mut command = Command::new("goose"); + command + .arg("run") + .arg("--recipe") + .arg(path) + .arg("--no-session"); + + for (key, value) in command_parameters { + let key_str = key.to_string(); + let value_str = value.as_str().unwrap_or(&value.to_string()).to_string(); + command + .arg("--params") + .arg(format!("{}={}", key_str, value_str)); + } command.stdout(Stdio::piped()); command.stderr(Stdio::piped()); - Ok((command, output_identifier)) + + Ok((command, format!("sub-recipe {}", sub_recipe_name))) } async fn run_command( @@ -174,7 +187,7 @@ async fn run_command( if let Err(e) = child.kill().await { tracing::warn!("Failed to kill child process: {}", e); } - // Abort the output reading tasks + stdout_task.abort(); stderr_task.abort(); return Err("Command cancelled".to_string()); diff --git a/crates/goose/src/agents/subagent_execution_tool/tasks_manager.rs b/crates/goose/src/agents/subagent_execution_tool/tasks_manager.rs index 4864994b7a0d..b416518a9cbc 100644 --- a/crates/goose/src/agents/subagent_execution_tool/tasks_manager.rs +++ b/crates/goose/src/agents/subagent_execution_tool/tasks_manager.rs @@ -4,6 +4,8 @@ use std::sync::Arc; use tokio::sync::RwLock; use crate::agents::subagent_execution_tool::task_types::Task; +#[cfg(test)] +use crate::agents::subagent_execution_tool::task_types::TaskType; #[derive(Debug, Clone)] pub struct TasksManager { @@ -60,7 +62,7 @@ mod tests { fn create_test_task(id: &str, sub_recipe_name: &str) -> Task { Task { id: id.to_string(), - task_type: "sub_recipe".to_string(), + task_type: TaskType::SubRecipe, payload: json!({ "sub_recipe": { "name": sub_recipe_name, diff --git a/crates/goose/src/agents/subagent_execution_tool/utils/tests.rs b/crates/goose/src/agents/subagent_execution_tool/utils/tests.rs index 253d28ebff00..3e5c5cd291a2 100644 --- a/crates/goose/src/agents/subagent_execution_tool/utils/tests.rs +++ b/crates/goose/src/agents/subagent_execution_tool/utils/tests.rs @@ -1,4 +1,4 @@ -use crate::agents::subagent_execution_tool::task_types::{Task, TaskInfo, TaskStatus}; +use crate::agents::subagent_execution_tool::task_types::{Task, TaskInfo, TaskStatus, TaskType}; use crate::agents::subagent_execution_tool::utils::{ count_by_status, get_task_name, strip_ansi_codes, }; @@ -23,7 +23,7 @@ mod test_get_task_name { fn test_extracts_sub_recipe_name() { let sub_recipe_task = Task { id: "task_1".to_string(), - task_type: "sub_recipe".to_string(), + task_type: TaskType::SubRecipe, payload: json!({ "sub_recipe": { "name": "my_recipe", @@ -38,14 +38,14 @@ mod test_get_task_name { } #[test] - fn falls_back_to_task_id_for_text_instruction() { - let text_task = Task { + fn falls_back_to_task_id_for_inline_recipe() { + let inline_task = Task { id: "task_2".to_string(), - task_type: "text_instruction".to_string(), - payload: json!({"text_instruction": "do something"}), + task_type: TaskType::InlineRecipe, + payload: json!({"recipe": {"instructions": "do something"}}), }; - let task_info = create_task_info_with_defaults(text_task, TaskStatus::Pending); + let task_info = create_task_info_with_defaults(inline_task, TaskStatus::Pending); assert_eq!(get_task_name(&task_info), "task_2"); } @@ -54,7 +54,7 @@ mod test_get_task_name { fn falls_back_to_task_id_when_sub_recipe_name_missing() { let malformed_task = Task { id: "task_3".to_string(), - task_type: "sub_recipe".to_string(), + task_type: TaskType::SubRecipe, payload: json!({ "sub_recipe": { "recipe_path": "/path/to/recipe" @@ -72,7 +72,7 @@ mod test_get_task_name { fn falls_back_to_task_id_when_sub_recipe_missing() { let malformed_task = Task { id: "task_4".to_string(), - task_type: "sub_recipe".to_string(), + task_type: TaskType::SubRecipe, payload: json!({}), // missing "sub_recipe" field }; @@ -88,7 +88,7 @@ mod count_by_status { fn create_test_task(id: &str, status: TaskStatus) -> TaskInfo { let task = Task { id: id.to_string(), - task_type: "test".to_string(), + task_type: TaskType::InlineRecipe, payload: json!({}), }; create_task_info_with_defaults(task, status) diff --git a/crates/goose/src/agents/subagent_handler.rs b/crates/goose/src/agents/subagent_handler.rs index 80e36f41aa3e..c2cb724a8eb0 100644 --- a/crates/goose/src/agents/subagent_handler.rs +++ b/crates/goose/src/agents/subagent_handler.rs @@ -7,6 +7,15 @@ use rmcp::model::{ErrorCode, ErrorData}; pub async fn run_complete_subagent_task( text_instruction: String, task_config: TaskConfig, +) -> Result { + run_complete_subagent_task_with_options(text_instruction, task_config, false).await +} + +/// Standalone function to run a complete subagent task with output options +pub async fn run_complete_subagent_task_with_options( + text_instruction: String, + task_config: TaskConfig, + return_last_only: bool, ) -> Result { // Create the subagent with the parent agent's provider let subagent = SubAgent::new(task_config.clone()).await.map_err(|e| { @@ -22,46 +31,65 @@ pub async fn run_complete_subagent_task( .reply_subagent(text_instruction, task_config) .await?; - // Extract all text content from all messages - let all_text_content: Vec = messages - .iter() - .flat_map(|message| { - message.content.iter().filter_map(|content| { - match content { + // Extract text content based on return_last_only flag + let response_text = if return_last_only { + // Get only the last message's text content + messages + .messages() + .last() + .and_then(|message| { + message.content.iter().find_map(|content| match content { crate::conversation::message::MessageContent::Text(text_content) => { Some(text_content.text.clone()) } - crate::conversation::message::MessageContent::ToolResponse(tool_response) => { - // Extract text from tool response - if let Ok(contents) = &tool_response.tool_result { - let texts: Vec = contents - .iter() - .filter_map(|content| { - if let rmcp::model::RawContent::Text(raw_text_content) = - &content.raw - { - Some(raw_text_content.text.clone()) - } else { - None - } - }) - .collect(); - if !texts.is_empty() { - Some(format!("Tool result: {}", texts.join("\n"))) + _ => None, + }) + }) + .unwrap_or_else(|| String::from("No text content in last message")) + } else { + // Extract all text content from all messages (original behavior) + let all_text_content: Vec = messages + .iter() + .flat_map(|message| { + message.content.iter().filter_map(|content| { + match content { + crate::conversation::message::MessageContent::Text(text_content) => { + Some(text_content.text.clone()) + } + crate::conversation::message::MessageContent::ToolResponse( + tool_response, + ) => { + // Extract text from tool response + if let Ok(contents) = &tool_response.tool_result { + let texts: Vec = contents + .iter() + .filter_map(|content| { + if let rmcp::model::RawContent::Text(raw_text_content) = + &content.raw + { + Some(raw_text_content.text.clone()) + } else { + None + } + }) + .collect(); + if !texts.is_empty() { + Some(format!("Tool result: {}", texts.join("\n"))) + } else { + None + } } else { None } - } else { - None } + _ => None, } - _ => None, - } + }) }) - }) - .collect(); + .collect(); - let response_text = all_text_content.join("\n"); + all_text_content.join("\n") + }; // Return the result Ok(response_text) diff --git a/crates/goose/src/agents/subagent_task_config.rs b/crates/goose/src/agents/subagent_task_config.rs index 5a0046292967..5cdcf76b0228 100644 --- a/crates/goose/src/agents/subagent_task_config.rs +++ b/crates/goose/src/agents/subagent_task_config.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use uuid::Uuid; /// Default maximum number of turns for task execution -pub const DEFAULT_SUBAGENT_MAX_TURNS: usize = 5; +pub const DEFAULT_SUBAGENT_MAX_TURNS: usize = 25; /// Environment variable name for configuring max turns pub const GOOSE_SUBAGENT_MAX_TURNS_ENV_VAR: &str = "GOOSE_SUBAGENT_MAX_TURNS"; @@ -16,6 +16,7 @@ pub struct TaskConfig { pub id: String, pub provider: Option>, pub max_turns: Option, + pub extensions: Option>, } impl fmt::Debug for TaskConfig { @@ -24,6 +25,7 @@ impl fmt::Debug for TaskConfig { .field("id", &self.id) .field("provider", &"") .field("max_turns", &self.max_turns) + .field("extensions", &self.extensions) .finish() } } @@ -40,6 +42,7 @@ impl TaskConfig { .and_then(|val| val.parse::().ok()) .unwrap_or(DEFAULT_SUBAGENT_MAX_TURNS), ), + extensions: None, } } diff --git a/crates/goose/src/prompts/subagent_system.md b/crates/goose/src/prompts/subagent_system.md index 2dd1983fdb05..6d45e00cd4e2 100644 --- a/crates/goose/src/prompts/subagent_system.md +++ b/crates/goose/src/prompts/subagent_system.md @@ -31,5 +31,6 @@ You have access to {{tool_count}} tools: {{available_tools}} - **Completion**: Clearly indicate when your task is complete - **Scope**: Stay focused on your assigned task - **Format**: Use Markdown formatting for responses +- **Summarization**: If asked for a summary or report of your work, that should be the last message you generate -Remember: You are part of a larger system. Your specialized focus helps the main agent handle multiple concerns efficiently. Complete your task efficiently with less tool usage. \ No newline at end of file +Remember: You are part of a larger system. Your specialized focus helps the main agent handle multiple concerns efficiently. Complete your task efficiently with less tool usage. diff --git a/crates/goose/src/prompts/system.md b/crates/goose/src/prompts/system.md index 1bcf22993c72..62729a5e81d4 100644 --- a/crates/goose/src/prompts/system.md +++ b/crates/goose/src/prompts/system.md @@ -40,21 +40,30 @@ No extensions are defined. You should let the user know that they should add ext # Task Management -- Required — use `todo__read` and `todo__write` for any task with 2+ steps, multiple files/components, or uncertain scope. Skipping them is an error. -- Start — `todo__read`, then `todo__write` a brief checklist (Markdown checkboxes). -- During — after each major action, update via `todo__write`: mark done, add/edit items, note blockers/dependencies. -- Finish — ensure every item is checked, or clearly list what remains. -- Overwrite warning — `todo__write` replaces the entire list; always read before writing. It is an error to not read before writing. -- Quality — keep items short, specific, and action‑oriented. +- Use `todo__read` and `todo__write` for tasks with 2+ steps, multiple files/components, or uncertain scope +- Workflow — Start: read → write checklist | During: read → update progress | End: verify all complete +- Warning — `todo__write` overwrites entirely; always `todo__read` first (skipping is an error) +- Keep items short, specific, action-oriented +- Not using the todo tools for complex tasks is an error Template: ```markdown - [ ] Implement feature X - [ ] Update API - [ ] Write tests + - [ ] Run tests (subagent in parallel) + - [ ] Run lint (subagent in parallel) - [ ] Blocked: waiting on credentials ``` +Execute via subagent by default — only handle directly when step-by-step visibility is essential. +- Delegate via `dynamic_task__create_task` for: result-only operations, parallelizable work, multi-part requests, verification, exploration +- Parallel subagents for multiple operations, single subagents for independent work +- Explore solutions in parallel — launch parallel subagents with different approaches (if non-interfering) +- Provide all needed context — subagents cannot see your context +- Use extension filters to limit resource access +- Use return_last_only when only a summary or simple answer is required — inform subagent of this choice. + # Response Guidelines - Use Markdown formatting for all responses. diff --git a/crates/goose/tests/dynamic_task_tools_tests.rs b/crates/goose/tests/dynamic_task_tools_tests.rs new file mode 100644 index 000000000000..286f1b456403 --- /dev/null +++ b/crates/goose/tests/dynamic_task_tools_tests.rs @@ -0,0 +1,532 @@ +use goose::agents::recipe_tools::dynamic_task_tools::{ + create_dynamic_task, task_params_to_inline_recipe, +}; +use serde_json::json; + +#[cfg(test)] +mod tests { + use super::*; + + // Helper function to create a list of loaded extensions for testing + fn test_loaded_extensions() -> Vec { + vec!["developer".to_string(), "memory".to_string()] + } + + #[test] + fn test_minimal_task_with_instructions() { + let params = json!({ + "instructions": "Test task" + }); + + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert_eq!(recipe.instructions, Some("Test task".to_string())); + assert_eq!(recipe.title, "Dynamic Task"); + assert_eq!(recipe.description, "Inline recipe task"); + } + + #[test] + fn test_minimal_task_with_prompt() { + let params = json!({ + "prompt": "Test prompt" + }); + + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert_eq!(recipe.prompt, Some("Test prompt".to_string())); + } + + #[test] + fn test_missing_required_fields() { + let params = json!({ + "title": "Test" + }); + + let result = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("instructions' or 'prompt")); + } + + #[test] + fn test_with_recipe_fields() { + let params = json!({ + "instructions": "Test", + "title": "Custom Title", + "description": "Custom Description", + "retry": { + "max_retries": 3, + "checks": [ + { + "type": "shell", + "command": "echo test" + } + ] + }, + "response": { + "json_schema": { + "type": "object" + } + } + }); + + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert_eq!(recipe.title, "Custom Title"); + assert_eq!(recipe.description, "Custom Description"); + assert!(recipe.retry.is_some()); + assert!(recipe.response.is_some()); + + // Verify retry config details + let retry = recipe.retry.unwrap(); + assert_eq!(retry.max_retries, 3); + assert_eq!(retry.checks.len(), 1); + } + + #[test] + fn test_security_validation() { + let params = json!({ + "instructions": format!("Test{}", '\u{E0041}') // Harmful Unicode tag + }); + + let result = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("harmful")); + } + + #[tokio::test] + async fn test_create_multiple_tasks() { + use goose::agents::subagent_execution_tool::tasks_manager::TasksManager; + + let tasks_manager = TasksManager::new(); + let params = json!({ + "task_parameters": [ + {"instructions": "Task 1"}, + {"prompt": "Task 2"} + ] + }); + + let result = create_dynamic_task(params, &tasks_manager, test_loaded_extensions()).await; + + // Check that the result is successful by awaiting the future + let tool_result = result.result.await; + assert!(tool_result.is_ok()); + let contents = tool_result.unwrap(); + assert!(!contents.is_empty()); + + // Parse the returned JSON to verify task creation + if let Some(text_content) = contents.first().and_then(|c| c.as_text()) { + let task_payload: serde_json::Value = serde_json::from_str(&text_content.text).unwrap(); + assert!(task_payload.get("task_ids").is_some()); + let task_ids = task_payload.get("task_ids").unwrap().as_array().unwrap(); + assert_eq!(task_ids.len(), 2); + } + } + + #[test] + fn test_return_last_only_flag() { + let params_with_flag = json!({ + "instructions": "Test task", + "return_last_only": true + }); + + let recipe = + task_params_to_inline_recipe(¶ms_with_flag, &test_loaded_extensions()).unwrap(); + assert_eq!(recipe.instructions, Some("Test task".to_string())); + + // The flag should not affect the recipe itself, only the task payload + // We can't test the task creation here without async context + + let params_without_flag = json!({ + "instructions": "Test task" + }); + + let recipe2 = + task_params_to_inline_recipe(¶ms_without_flag, &test_loaded_extensions()).unwrap(); + assert_eq!(recipe2.instructions, Some("Test task".to_string())); + } + + #[tokio::test] + async fn test_text_instruction_not_supported() { + use goose::agents::subagent_execution_tool::tasks_manager::TasksManager; + + let tasks_manager = TasksManager::new(); + let params = json!({ + "task_parameters": [ + {"text_instruction": "Legacy task"} + ] + }); + + let result = create_dynamic_task(params, &tasks_manager, test_loaded_extensions()).await; + + // Check that the result fails since text_instruction is no longer supported + let tool_result = result.result.await; + assert!(tool_result.is_err()); + + // Verify the error message indicates missing required fields + if let Err(err) = tool_result { + let error_msg = err.message.to_string(); + assert!(error_msg.contains("instructions") || error_msg.contains("prompt")); + } + } + + #[test] + fn test_with_extensions() { + let params = json!({ + "instructions": "Test", + "extensions": [ + { + "type": "builtin", + "name": "developer" + } + ] + }); + + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert!(recipe.extensions.is_some()); + let extensions = recipe.extensions.unwrap(); + assert_eq!(extensions.len(), 1); + } + + #[test] + fn test_with_context_and_activities() { + let params = json!({ + "instructions": "Test", + "context": ["context1", "context2"], + "activities": ["activity1", "activity2"] + }); + + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert!(recipe.context.is_some()); + assert!(recipe.activities.is_some()); + assert_eq!(recipe.context.unwrap(), vec!["context1", "context2"]); + assert_eq!(recipe.activities.unwrap(), vec!["activity1", "activity2"]); + } + + #[test] + fn test_invalid_retry_config() { + // Test with max_retries = 0 (invalid) + let params = json!({ + "instructions": "Test", + "retry": { + "max_retries": 0, // Invalid: must be > 0 + "checks": [] + } + }); + + let result = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Invalid retry config")); + } + + #[test] + fn test_invalid_retry_config_missing_checks() { + // Test with missing required field 'checks' + let params = json!({ + "instructions": "Test", + "retry": { + "max_retries": 3 + // Missing 'checks' field + } + }); + + let result = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()); + // This should fail during deserialization since 'checks' is required + assert!(result.is_ok()); // But retry field will be None due to failed deserialization + let recipe = result.unwrap(); + assert!(recipe.retry.is_none()); + } + + // Additional edge case tests + + #[test] + fn test_both_instructions_and_prompt() { + // Test that both instructions and prompt can be provided + let params = json!({ + "instructions": "Test instructions", + "prompt": "Test prompt" + }); + + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert_eq!(recipe.instructions, Some("Test instructions".to_string())); + assert_eq!(recipe.prompt, Some("Test prompt".to_string())); + } + + #[test] + fn test_empty_task_parameters_array() { + // This test is for the create_dynamic_task function + // We can't test it here without async, but we document the expected behavior + // Empty task_parameters array should return an error + } + + #[test] + fn test_invalid_json_in_optional_fields() { + // Test that invalid JSON in optional fields is gracefully ignored + let params = json!({ + "instructions": "Test", + "settings": "not an object", // Invalid: should be object + "extensions": "not an array", // Invalid: should be array + "context": {"not": "an array"}, // Invalid: should be array + "activities": 123 // Invalid: should be array + }); + + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert_eq!(recipe.instructions, Some("Test".to_string())); + // Invalid fields should be ignored (None) + assert!(recipe.settings.is_none()); + assert!(recipe.extensions.is_none()); + assert!(recipe.context.is_none()); + assert!(recipe.activities.is_none()); + } + + #[test] + fn test_with_settings() { + let params = json!({ + "instructions": "Test", + "settings": { + "goose_provider": "openai", + "goose_model": "gpt-4", + "temperature": 0.7 + } + }); + + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert!(recipe.settings.is_some()); + let settings = recipe.settings.unwrap(); + assert_eq!(settings.goose_provider, Some("openai".to_string())); + assert_eq!(settings.goose_model, Some("gpt-4".to_string())); + assert_eq!(settings.temperature, Some(0.7)); + } + + #[test] + fn test_with_parameters() { + let params = json!({ + "instructions": "Test", + "parameters": [ + { + "key": "test_param", + "input_type": "string", + "requirement": "required", + "description": "A test parameter" + } + ] + }); + + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert!(recipe.parameters.is_some()); + let parameters = recipe.parameters.unwrap(); + assert_eq!(parameters.len(), 1); + assert_eq!(parameters[0].key, "test_param"); + } + + #[test] + fn test_empty_strings_for_required_fields() { + // Empty strings should be valid for instructions/prompt + let params = json!({ + "instructions": "" + }); + + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert_eq!(recipe.instructions, Some("".to_string())); + } + + #[test] + fn test_very_long_instruction() { + // Test with a very long instruction string + let long_instruction = "a".repeat(10000); + let params = json!({ + "instructions": long_instruction.clone() + }); + + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert_eq!(recipe.instructions, Some(long_instruction)); + } + + #[tokio::test] + async fn test_mixed_valid_and_invalid_tasks() { + use goose::agents::subagent_execution_tool::tasks_manager::TasksManager; + + let tasks_manager = TasksManager::new(); + let params = json!({ + "task_parameters": [ + {"instructions": "Valid task"}, + {"title": "Invalid - missing instruction"}, // This should cause error + ] + }); + + let result = create_dynamic_task(params, &tasks_manager, test_loaded_extensions()).await; + + // Should fail on the invalid task + let tool_result = result.result.await; + assert!(tool_result.is_err()); + } + + #[test] + fn test_unicode_in_non_instruction_fields() { + // Unicode tags should be allowed in non-instruction fields + let params = json!({ + "instructions": "Test", + "title": format!("Title with unicode {}", '\u{E0041}'), + "description": format!("Description with unicode {}", '\u{E0041}') + }); + + // This should succeed - only instructions/prompt/activities are checked for security + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert!(recipe.title.contains('\u{E0041}')); + assert!(recipe.description.contains('\u{E0041}')); + } + + #[test] + fn test_extension_shortnames() { + // Test that extension shortnames are properly resolved + // Note: This test now depends on actual config, so it may not find all extensions + // if they're not configured in the test environment + let loaded_exts = vec!["developer".to_string(), "memory".to_string()]; + let params = json!({ + "instructions": "Test", + "extensions": ["developer", "memory"] + }); + + let recipe = task_params_to_inline_recipe(¶ms, &loaded_exts).unwrap(); + assert!(recipe.extensions.is_some()); + let extensions = recipe.extensions.unwrap(); + // We can't guarantee both extensions exist in config during tests + // Just check that we got some extensions and they have the right structure + assert!(extensions.len() <= 2); + if !extensions.is_empty() { + // Check that the first one is a valid ExtensionConfig + assert!(matches!( + &extensions[0], + goose::agents::extension::ExtensionConfig::Builtin { .. } + | goose::agents::extension::ExtensionConfig::Stdio { .. } + | goose::agents::extension::ExtensionConfig::Sse { .. } + | goose::agents::extension::ExtensionConfig::StreamableHttp { .. } + | goose::agents::extension::ExtensionConfig::Frontend { .. } + | goose::agents::extension::ExtensionConfig::InlinePython { .. } + )); + } + } + + #[test] + fn test_mixed_extension_formats() { + // Test mixing shortnames and full configs + // Note: Shortnames depend on config being present, which may not exist in CI + let loaded_exts = vec!["developer".to_string(), "memory".to_string()]; + let params = json!({ + "instructions": "Test", + "extensions": [ + "developer", // Shortname - may not resolve in CI + { + "type": "stdio", + "name": "custom", + "cmd": "echo", + "args": ["test"] + } + ] + }); + + let recipe = task_params_to_inline_recipe(¶ms, &loaded_exts).unwrap(); + assert!(recipe.extensions.is_some()); + let extensions = recipe.extensions.unwrap(); + // At minimum we should get the full config (stdio), shortname may not resolve + assert!(extensions.len() >= 1 && extensions.len() <= 2); + // The last one should always be the stdio config we provided + if let Some(last) = extensions.last() { + match last { + goose::agents::extension::ExtensionConfig::Stdio { name, .. } => { + assert_eq!(name, "custom"); + } + _ => { + // If we got 2 extensions, the second should be stdio + if extensions.len() == 2 { + panic!("Expected stdio extension config for 'custom'"); + } + } + } + } + } + + #[test] + fn test_unknown_extension_shortname() { + // Test that unknown extension shortnames are skipped while valid configs are kept + let loaded_exts = vec!["developer".to_string()]; + let params = json!({ + "instructions": "Test", + "extensions": [ + "unknown_extension_1", // Full config should always work + { + "type": "builtin", + "name": "test_builtin", + "display_name": "Test Builtin", + "description": "Test extension" + }, + "unknown_extension_2" // Should be skipped + ] + }); + + let recipe = task_params_to_inline_recipe(¶ms, &loaded_exts).unwrap(); + assert!(recipe.extensions.is_some()); + let extensions = recipe.extensions.unwrap(); + // Should only get the full config, unknown shortnames should be skipped + assert_eq!(extensions.len(), 1); + // Verify it's the builtin we provided + match &extensions[0] { + goose::agents::extension::ExtensionConfig::Builtin { name, .. } => { + assert_eq!(name, "test_builtin"); + } + _ => panic!("Expected builtin extension config"), + } + } + + #[test] + fn test_empty_extensions_array() { + // Test that an empty extensions array results in no extensions + let loaded_exts = vec!["developer".to_string(), "memory".to_string()]; + let params = json!({ + "instructions": "Test", + "extensions": [] + }); + + let recipe = task_params_to_inline_recipe(¶ms, &loaded_exts).unwrap(); + assert!(recipe.extensions.is_some()); + let extensions = recipe.extensions.unwrap(); + // Empty array should mean no extensions + assert_eq!(extensions.len(), 0); + } + + #[test] + fn test_omitted_extensions_field() { + // Test that omitting the extensions field results in None (use all) + let loaded_exts = vec!["developer".to_string(), "memory".to_string()]; + let params = json!({ + "instructions": "Test" + // No extensions field + }); + + let recipe = task_params_to_inline_recipe(¶ms, &loaded_exts).unwrap(); + // When extensions field is omitted, recipe.extensions should be None + assert!(recipe.extensions.is_none()); + } + + #[test] + fn test_null_values_in_optional_fields() { + // Test that null values in optional fields are handled gracefully + let params = json!({ + "instructions": "Test", + "title": null, + "description": null, + "extensions": null, + "settings": null + }); + + let recipe = task_params_to_inline_recipe(¶ms, &test_loaded_extensions()).unwrap(); + assert_eq!(recipe.instructions, Some("Test".to_string())); + // Null values should use defaults or be None + assert_eq!(recipe.title, "Dynamic Task"); // Should use default + assert_eq!(recipe.description, "Inline recipe task"); // Should use default + assert!(recipe.extensions.is_none()); + assert!(recipe.settings.is_none()); + } +} diff --git a/crates/goose/tests/task_types_tests.rs b/crates/goose/tests/task_types_tests.rs new file mode 100644 index 000000000000..970d0286bd5d --- /dev/null +++ b/crates/goose/tests/task_types_tests.rs @@ -0,0 +1,126 @@ +use goose::agents::subagent_execution_tool::task_types::{Task, TaskType}; +use serde_json::json; + +#[test] +fn test_task_type_serialization() { + // Test that TaskType serializes to the expected string format + assert_eq!( + serde_json::to_string(&TaskType::InlineRecipe).unwrap(), + "\"inline_recipe\"" + ); + assert_eq!( + serde_json::to_string(&TaskType::SubRecipe).unwrap(), + "\"sub_recipe\"" + ); +} + +#[test] +fn test_task_type_deserialization() { + // Test that strings deserialize to the correct TaskType variants + assert_eq!( + serde_json::from_str::("\"inline_recipe\"").unwrap(), + TaskType::InlineRecipe + ); + assert_eq!( + serde_json::from_str::("\"sub_recipe\"").unwrap(), + TaskType::SubRecipe + ); +} + +#[test] +fn test_task_serialization_with_enum() { + let task = Task { + id: "test-id".to_string(), + task_type: TaskType::InlineRecipe, + payload: json!({"recipe": "test"}), + }; + + let serialized = serde_json::to_value(&task).unwrap(); + assert_eq!(serialized["id"], "test-id"); + assert_eq!(serialized["task_type"], "inline_recipe"); + assert_eq!(serialized["payload"]["recipe"], "test"); +} + +#[test] +fn test_task_deserialization_with_string() { + // Test backward compatibility - JSON with string task_type should deserialize + let json_str = r#"{ + "id": "test-id", + "task_type": "sub_recipe", + "payload": {"sub_recipe": {"name": "test"}} + }"#; + + let task: Task = serde_json::from_str(json_str).unwrap(); + assert_eq!(task.id, "test-id"); + assert_eq!(task.task_type, TaskType::SubRecipe); +} + +#[test] +fn test_task_type_display() { + assert_eq!(TaskType::InlineRecipe.to_string(), "inline_recipe"); + assert_eq!(TaskType::SubRecipe.to_string(), "sub_recipe"); +} + +#[test] +fn test_task_methods_with_sub_recipe() { + let task = Task { + id: "test-1".to_string(), + task_type: TaskType::SubRecipe, + payload: json!({ + "sub_recipe": { + "name": "test_recipe", + "recipe_path": "/path/to/recipe", + "command_parameters": {"key": "value"}, + "sequential_when_repeated": true + } + }), + }; + + assert!(task.get_sub_recipe().is_some()); + assert_eq!(task.get_sub_recipe_name(), Some("test_recipe")); + assert_eq!(task.get_sub_recipe_path(), Some("/path/to/recipe")); + assert!(task.get_command_parameters().is_some()); + assert!(task.get_sequential_when_repeated()); +} + +#[test] +fn test_task_methods_with_inline_recipe() { + let task = Task { + id: "test-3".to_string(), + task_type: TaskType::InlineRecipe, + payload: json!({ + "recipe": { + "instructions": "Test instructions" + }, + "return_last_only": true + }), + }; + + assert!(task.get_sub_recipe().is_none()); + assert!(task.get_sub_recipe_name().is_none()); + assert!(task.get_sub_recipe_path().is_none()); + assert!(task.get_command_parameters().is_none()); + assert!(!task.get_sequential_when_repeated()); +} + +#[test] +fn test_invalid_task_type_deserialization() { + // Test that invalid task_type strings fail to deserialize + let result = serde_json::from_str::("\"invalid_type\""); + assert!(result.is_err()); +} + +#[test] +fn test_task_with_missing_fields() { + let task = Task { + id: "test-4".to_string(), + task_type: TaskType::SubRecipe, + payload: json!({}), // Missing sub_recipe field + }; + + assert!(task.get_sub_recipe().is_none()); + assert!(task.get_sub_recipe_name().is_none()); + assert!(task.get_sub_recipe_path().is_none()); + assert!(task.get_command_parameters().is_none()); + assert!(!task.get_sequential_when_repeated()); +}