diff --git a/crates/coding-agent-loop/src/subagent.rs b/crates/coding-agent-loop/src/subagent.rs index a61ca7921..196e8e871 100644 --- a/crates/coding-agent-loop/src/subagent.rs +++ b/crates/coding-agent-loop/src/subagent.rs @@ -1,6 +1,7 @@ use crate::error::AgentError; use crate::session::Session; use crate::tool_registry::RegisteredTool; +use crate::tools::required_str; use crate::types::Turn; use std::collections::{HashMap, VecDeque}; use std::sync::atomic::{AtomicBool, Ordering}; @@ -17,18 +18,17 @@ pub struct SubAgentResult { } pub struct SubAgent { + #[allow(dead_code)] id: String, + #[allow(dead_code)] depth: usize, task: Option>>, followup_queue: Arc>>, abort_flag: Arc, } +#[cfg(test)] impl SubAgent { - pub fn id(&self) -> &str { - &self.id - } - pub fn depth(&self) -> usize { self.depth } @@ -65,24 +65,16 @@ impl SubAgentManager { let abort_flag = session.abort_flag_handle(); let task = tokio::spawn(async move { - let result = session.process_input(&task_prompt).await; + session.process_input(&task_prompt).await?; let turns = session.history().turns(); - let turns_used = turns.len(); - let last_text = turns.iter().rev().find_map(|t| { - if let Turn::Assistant { content, .. } = t { - Some(content.clone()) - } else { - None - } + let last_text = turns.iter().rev().find_map(|t| match t { + Turn::Assistant { content, .. } => Some(content.clone()), + _ => None, }); - let success = result.is_ok(); - if let Err(e) = result { - return Err(e); - } Ok(SubAgentResult { output: last_text.unwrap_or_default(), - success, - turns_used, + success: true, + turns_used: turns.len(), }) }); @@ -145,6 +137,7 @@ impl SubAgentManager { Ok(()) } + #[cfg(test)] pub fn get(&self, agent_id: &str) -> Option<&SubAgent> { self.agents.get(agent_id) } @@ -186,10 +179,7 @@ pub fn make_spawn_agent_tool( let manager = manager.clone(); let session_factory = session_factory.clone(); Box::pin(async move { - let task = args - .get("task") - .and_then(|v| v.as_str()) - .ok_or_else(|| "Missing required parameter: task".to_string())?; + let task = required_str(&args, "task")?; // Extract optional max_turns parameter #[allow(clippy::cast_possible_truncation)] @@ -234,14 +224,8 @@ pub fn make_send_input_tool( executor: Arc::new(move |args, _env| { let manager = manager.clone(); Box::pin(async move { - let agent_id = args - .get("agent_id") - .and_then(|v| v.as_str()) - .ok_or_else(|| "Missing required parameter: agent_id".to_string())?; - let message = args - .get("message") - .and_then(|v| v.as_str()) - .ok_or_else(|| "Missing required parameter: message".to_string())?; + let agent_id = required_str(&args, "agent_id")?; + let message = required_str(&args, "message")?; let mgr = manager.lock().await; mgr.send_input(agent_id, message)?; @@ -272,10 +256,7 @@ pub fn make_wait_tool( executor: Arc::new(move |args, _env| { let manager = manager.clone(); Box::pin(async move { - let agent_id = args - .get("agent_id") - .and_then(|v| v.as_str()) - .ok_or_else(|| "Missing required parameter: agent_id".to_string())?; + let agent_id = required_str(&args, "agent_id")?; let mut mgr = manager.lock().await; let result = mgr.wait(agent_id).await?; @@ -309,10 +290,7 @@ pub fn make_close_agent_tool( executor: Arc::new(move |args, _env| { let manager = manager.clone(); Box::pin(async move { - let agent_id = args - .get("agent_id") - .and_then(|v| v.as_str()) - .ok_or_else(|| "Missing required parameter: agent_id".to_string())?; + let agent_id = required_str(&args, "agent_id")?; let mut mgr = manager.lock().await; mgr.close(agent_id)?; diff --git a/crates/coding-agent-loop/src/tools.rs b/crates/coding-agent-loop/src/tools.rs index 405a7c65c..a7104b8f3 100644 --- a/crates/coding-agent-loop/src/tools.rs +++ b/crates/coding-agent-loop/src/tools.rs @@ -5,6 +5,12 @@ use std::fmt::Write; use std::sync::Arc; use unified_llm::types::ToolDefinition; +pub(crate) fn required_str<'a>(args: &'a serde_json::Value, key: &str) -> Result<&'a str, String> { + args.get(key) + .and_then(|v| v.as_str()) + .ok_or_else(|| format!("Missing required parameter: {key}")) +} + #[must_use] pub fn make_read_file_tool() -> RegisteredTool { RegisteredTool { @@ -23,9 +29,7 @@ pub fn make_read_file_tool() -> RegisteredTool { }, executor: Arc::new(|args, env| { Box::pin(async move { - let file_path = args["file_path"] - .as_str() - .ok_or_else(|| "file_path is required".to_string())?; + let file_path = required_str(&args, "file_path")?; let offset = args.get("offset").and_then(serde_json::Value::as_u64); let limit = args.get("limit").and_then(serde_json::Value::as_u64); @@ -58,12 +62,8 @@ pub fn make_write_file_tool() -> RegisteredTool { }, executor: Arc::new(|args, env| { Box::pin(async move { - let file_path = args["file_path"] - .as_str() - .ok_or_else(|| "file_path is required".to_string())?; - let content = args["content"] - .as_str() - .ok_or_else(|| "content is required".to_string())?; + let file_path = required_str(&args, "file_path")?; + let content = required_str(&args, "content")?; env.write_file(file_path, content).await?; Ok(format!("Successfully wrote to {file_path}")) @@ -91,15 +91,9 @@ pub fn make_edit_file_tool() -> RegisteredTool { }, executor: Arc::new(|args, env| { Box::pin(async move { - let file_path = args["file_path"] - .as_str() - .ok_or_else(|| "file_path is required".to_string())?; - let old_string = args["old_string"] - .as_str() - .ok_or_else(|| "old_string is required".to_string())?; - let new_string = args["new_string"] - .as_str() - .ok_or_else(|| "new_string is required".to_string())?; + let file_path = required_str(&args, "file_path")?; + let old_string = required_str(&args, "old_string")?; + let new_string = required_str(&args, "new_string")?; let replace_all = args .get("replace_all") .and_then(serde_json::Value::as_bool) @@ -165,9 +159,7 @@ pub fn make_shell_tool_with_config(config: &SessionConfig) -> RegisteredTool { }, executor: Arc::new(move |args, env| { Box::pin(async move { - let command = args["command"] - .as_str() - .ok_or_else(|| "command is required".to_string())?; + let command = required_str(&args, "command")?; let timeout_ms = args .get("timeout_ms") .and_then(serde_json::Value::as_u64) @@ -213,9 +205,7 @@ pub fn make_grep_tool() -> RegisteredTool { }, executor: Arc::new(|args, env| { Box::pin(async move { - let pattern = args["pattern"] - .as_str() - .ok_or_else(|| "pattern is required".to_string())?; + let pattern = required_str(&args, "pattern")?; let path = args .get("path") .and_then(serde_json::Value::as_str) @@ -261,9 +251,7 @@ pub fn make_glob_tool() -> RegisteredTool { }, executor: Arc::new(|args, env| { Box::pin(async move { - let pattern = args["pattern"] - .as_str() - .ok_or_else(|| "pattern is required".to_string())?; + let pattern = required_str(&args, "pattern")?; let path = args .get("path") .and_then(serde_json::Value::as_str); @@ -336,9 +324,7 @@ pub(crate) fn make_list_dir_tool() -> RegisteredTool { }, executor: Arc::new(|args, env| { Box::pin(async move { - let path = args["path"] - .as_str() - .ok_or_else(|| "path is required".to_string())?; + let path = required_str(&args, "path")?; #[allow(clippy::cast_possible_truncation)] let depth = args .get("depth")