diff --git a/.gitignore b/.gitignore index 6f57097..a0137e2 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ docs/ vendor/ +.worktrees/ diff --git a/crewforge-rs/Cargo.lock b/crewforge-rs/Cargo.lock index 454ba26..6aafc98 100644 --- a/crewforge-rs/Cargo.lock +++ b/crewforge-rs/Cargo.lock @@ -445,6 +445,7 @@ dependencies = [ "crossterm", "directories", "futures", + "glob", "predicates", "rand", "ratatui", @@ -465,6 +466,8 @@ dependencies = [ "unicode-width 0.2.0", "urlencoding", "uuid", + "walkdir", + "which", ] [[package]] @@ -613,6 +616,12 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "env_home" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c7f84e12ccf0a7ddc17a6c41c93326024c42920d7ee630d04950e6926645c0fe" + [[package]] name = "equivalent" version = "1.0.2" @@ -820,6 +829,12 @@ dependencies = [ "wasip3", ] +[[package]] +name = "glob" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280" + [[package]] name = "h2" version = "0.4.13" @@ -1880,6 +1895,15 @@ version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" +[[package]] +name = "same-file" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" +dependencies = [ + "winapi-util", +] + [[package]] name = "schannel" version = "0.1.28" @@ -2597,6 +2621,16 @@ dependencies = [ "libc", ] +[[package]] +name = "walkdir" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29790946404f91d9c5d06f9874efddea1dc06c5efe94541a7d6863108e3a5e4b" +dependencies = [ + "same-file", + "winapi-util", +] + [[package]] name = "want" version = "0.3.1" @@ -2756,6 +2790,18 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "which" +version = "7.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24d643ce3fd3e5b54854602a080f34fb10ab75e0b813ee32d00ca2b44fa74762" +dependencies = [ + "either", + "env_home", + "rustix 1.1.4", + "winsafe", +] + [[package]] name = "winapi" version = "0.3.9" @@ -2772,6 +2818,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" +[[package]] +name = "winapi-util" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" +dependencies = [ + "windows-sys 0.61.2", +] + [[package]] name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" @@ -3079,6 +3134,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6bbff5f0aada427a1e5a6da5f1f98158182f26556f345ac9e04d36d0ebed650" +[[package]] +name = "winsafe" +version = "0.0.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d135d17ab770252ad95e9a872d365cf3090e3be864a34ab46f48555993efc904" + [[package]] name = "wit-bindgen" version = "0.51.0" diff --git a/crewforge-rs/Cargo.toml b/crewforge-rs/Cargo.toml index 2813fab..f98f882 100644 --- a/crewforge-rs/Cargo.toml +++ b/crewforge-rs/Cargo.toml @@ -34,6 +34,9 @@ tui-textarea = { version = "0.7", features = ["crossterm"] } unicode-width = "0.2" uuid = { version = "1", features = ["v4"] } urlencoding = "2" +glob = "0.3" +walkdir = "2" +which = "7" [dev-dependencies] assert_cmd = "2" diff --git a/crewforge-rs/src/agent/dispatcher.rs b/crewforge-rs/src/agent/dispatcher.rs index f6923e6..b811a98 100644 --- a/crewforge-rs/src/agent/dispatcher.rs +++ b/crewforge-rs/src/agent/dispatcher.rs @@ -1,7 +1,5 @@ -use crate::provider::traits::{ChatMessage, ChatResponse, ConversationMessage, ToolResultMessage}; -use super::Tool; +use crate::provider::traits::{ChatResponse, ConversationMessage, ToolResultMessage}; use serde_json::Value; -use std::fmt::Write; #[derive(Debug, Clone)] pub struct ParsedToolCall { @@ -12,174 +10,53 @@ pub struct ParsedToolCall { #[derive(Debug, Clone)] pub struct ToolExecutionResult { + pub tool_result: super::ToolResult, pub name: String, - pub output: String, - pub success: bool, pub tool_call_id: Option, } -pub trait ToolDispatcher: Send + Sync { - fn parse_response(&self, response: &ChatResponse) -> (String, Vec); - fn format_results(&self, results: &[ToolExecutionResult]) -> ConversationMessage; - fn prompt_instructions(&self, tools: &[Box]) -> String; - fn should_send_tool_specs(&self) -> bool; +/// Parse native tool calls from a provider ChatResponse. +pub fn parse_response(response: &ChatResponse) -> (String, Vec) { + let text = response.text.clone().unwrap_or_default(); + let calls = response + .tool_calls + .iter() + .map(|tc| ParsedToolCall { + name: tc.name.clone(), + arguments: serde_json::from_str(&tc.arguments).unwrap_or_else(|e| { + tracing::warn!( + tool = %tc.name, + error = %e, + "Failed to parse native tool call arguments as JSON; defaulting to empty object" + ); + Value::Object(serde_json::Map::new()) + }), + tool_call_id: Some(tc.id.clone()), + }) + .collect(); + (text, calls) } -// ── XmlToolDispatcher ──────────────────────────────────────────────────────── - -#[derive(Default)] -pub struct XmlToolDispatcher; - -impl XmlToolDispatcher { - /// Parse `...` blocks out of LLM response text. - /// - /// Returns the stripped text (without tool_call tags) and a list of parsed calls. - fn parse_xml_tool_calls(response: &str) -> (String, Vec) { - let mut text_parts = Vec::new(); - let mut calls = Vec::new(); - let mut remaining = response; - - while let Some(start) = remaining.find("") { - let before = &remaining[..start]; - if !before.trim().is_empty() { - text_parts.push(before.trim().to_string()); - } - - if let Some(end) = remaining[start..].find("") { - let inner = &remaining[start + 11..start + end]; - match serde_json::from_str::(inner.trim()) { - Ok(parsed) => { - let name = parsed - .get("name") - .and_then(Value::as_str) - .unwrap_or("") - .to_string(); - if name.is_empty() { - remaining = &remaining[start + end + 12..]; - continue; - } - let arguments = parsed - .get("arguments") - .cloned() - .unwrap_or_else(|| Value::Object(serde_json::Map::new())); - calls.push(ParsedToolCall { - name, - arguments, - tool_call_id: None, - }); - } - Err(e) => { - tracing::warn!("Malformed JSON: {e}"); - } - } - remaining = &remaining[start + end + 12..]; - } else { - break; - } - } - - if !remaining.trim().is_empty() { - text_parts.push(remaining.trim().to_string()); - } - - (text_parts.join("\n"), calls) - } -} - -impl ToolDispatcher for XmlToolDispatcher { - fn parse_response(&self, response: &ChatResponse) -> (String, Vec) { - let text = response.text_or_empty(); - Self::parse_xml_tool_calls(text) - } - - fn format_results(&self, results: &[ToolExecutionResult]) -> ConversationMessage { - let mut content = String::new(); - for result in results { - let status = if result.success { "ok" } else { "error" }; - let _ = writeln!( - content, - "\n{}\n", - result.name, status, result.output - ); - } - ConversationMessage::Chat(ChatMessage::user(format!("[Tool results]\n{content}"))) - } - - fn prompt_instructions(&self, tools: &[Box]) -> String { - let mut instructions = String::new(); - instructions.push_str("## Tool Use Protocol\n\n"); - instructions - .push_str("To use a tool, wrap a JSON object in tags:\n\n"); - instructions.push_str( - "```\n\n{\"name\": \"tool_name\", \"arguments\": {\"param\": \"value\"}}\n\n```\n\n", - ); - instructions.push_str("### Available Tools\n\n"); - - for tool in tools { - let _ = writeln!( - instructions, - "- **{}**: {}\n Parameters: `{}`", - tool.name(), - tool.description(), - tool.parameters() - ); - } - - instructions - } - - fn should_send_tool_specs(&self) -> bool { - false - } -} - -// ── NativeToolDispatcher ───────────────────────────────────────────────────── - -pub struct NativeToolDispatcher; - -impl ToolDispatcher for NativeToolDispatcher { - fn parse_response(&self, response: &ChatResponse) -> (String, Vec) { - let text = response.text.clone().unwrap_or_default(); - let calls = response - .tool_calls - .iter() - .map(|tc| ParsedToolCall { - name: tc.name.clone(), - arguments: serde_json::from_str(&tc.arguments).unwrap_or_else(|e| { - tracing::warn!( - tool = %tc.name, - error = %e, - "Failed to parse native tool call arguments as JSON; defaulting to empty object" - ); - Value::Object(serde_json::Map::new()) - }), - tool_call_id: Some(tc.id.clone()), - }) - .collect(); - (text, calls) - } - - fn format_results(&self, results: &[ToolExecutionResult]) -> ConversationMessage { - let messages: Vec = results - .iter() - .map(|result| ToolResultMessage { +/// Format tool execution results as a ConversationMessage for history. +pub fn format_results(results: &[ToolExecutionResult]) -> ConversationMessage { + let messages: Vec = results + .iter() + .map(|result| { + let content = match (&result.tool_result.error, result.tool_result.output.is_empty()) { + (Some(err), true) => err.clone(), + (Some(err), false) => format!("{}\n{}", result.tool_result.output, err), + (None, _) => result.tool_result.output.clone(), + }; + ToolResultMessage { tool_call_id: result .tool_call_id .clone() .unwrap_or_else(|| "unknown".to_string()), - content: result.output.clone(), - }) - .collect(); - ConversationMessage::ToolResults(messages) - } - - fn prompt_instructions(&self, _tools: &[Box]) -> String { - String::new() - } - - fn should_send_tool_specs(&self) -> bool { - true - } + content, + } + }) + .collect(); + ConversationMessage::ToolResults(messages) } #[cfg(test)] @@ -188,95 +65,103 @@ mod tests { use crate::provider::traits::ToolCall; #[test] - fn xml_dispatcher_parses_tool_calls() { + fn parse_response_extracts_tool_calls() { let response = ChatResponse { - text: Some( - "Checking\n{\"name\":\"shell\",\"arguments\":{\"command\":\"ls\"}}" - .into(), - ), - tool_calls: vec![], + text: Some("ok".into()), + tool_calls: vec![ToolCall { + id: "tc1".into(), + name: "file_read".into(), + arguments: "{\"path\":\"a.txt\"}".into(), + }], usage: None, reasoning_content: None, }; - let dispatcher = XmlToolDispatcher; - let (_, calls) = dispatcher.parse_response(&response); + let (text, calls) = parse_response(&response); + assert_eq!(text, "ok"); assert_eq!(calls.len(), 1); - assert_eq!(calls[0].name, "shell"); + assert_eq!(calls[0].name, "file_read"); + assert_eq!(calls[0].tool_call_id.as_deref(), Some("tc1")); } #[test] - fn native_dispatcher_roundtrip() { + fn parse_response_bad_json_fallback() { let response = ChatResponse { - text: Some("ok".into()), + text: None, tool_calls: vec![ToolCall { - id: "tc1".into(), - name: "file_read".into(), - arguments: "{\"path\":\"a.txt\"}".into(), + id: "tc2".into(), + name: "tool_x".into(), + arguments: "not-json".into(), }], usage: None, reasoning_content: None, }; - let dispatcher = NativeToolDispatcher; - let (_, calls) = dispatcher.parse_response(&response); + let (_, calls) = parse_response(&response); assert_eq!(calls.len(), 1); - assert_eq!(calls[0].tool_call_id.as_deref(), Some("tc1")); + assert_eq!(calls[0].arguments, serde_json::json!({})); + } - let msg = dispatcher.format_results(&[ToolExecutionResult { - name: "file_read".into(), - output: "hello".into(), - success: true, - tool_call_id: Some("tc1".into()), + #[test] + fn format_results_preserves_tool_call_id() { + let msg = format_results(&[ToolExecutionResult { + tool_result: crate::agent::ToolResult { + success: true, + output: "ok".into(), + error: None, + }, + name: "shell".into(), + tool_call_id: Some("tc-1".into()), }]); match msg { ConversationMessage::ToolResults(results) => { assert_eq!(results.len(), 1); - assert_eq!(results[0].tool_call_id, "tc1"); + assert_eq!(results[0].tool_call_id, "tc-1"); + assert_eq!(results[0].content, "ok"); } - _ => panic!("expected tool results"), + _ => panic!("expected ToolResults variant"), } } #[test] - fn xml_format_results_contains_tool_result_tags() { - let dispatcher = XmlToolDispatcher; - let msg = dispatcher.format_results(&[ToolExecutionResult { + fn format_results_includes_error() { + let msg = format_results(&[ToolExecutionResult { + tool_result: crate::agent::ToolResult { + success: false, + output: "partial".into(), + error: Some("denied".into()), + }, name: "shell".into(), - output: "ok".into(), - success: true, - tool_call_id: None, + tool_call_id: Some("tc-2".into()), }]); - let rendered = match msg { - ConversationMessage::Chat(chat) => chat.content, - _ => String::new(), - }; - assert!(rendered.contains(" { + assert!(results[0].content.contains("denied")); + } + _ => panic!("expected ToolResults variant"), + } } #[test] - fn native_format_results_keeps_tool_call_id() { - let dispatcher = NativeToolDispatcher; - let msg = dispatcher.format_results(&[ToolExecutionResult { + fn format_results_error_with_empty_output_no_leading_newline() { + let msg = format_results(&[ToolExecutionResult { + tool_result: crate::agent::ToolResult::denied("access denied"), name: "shell".into(), - output: "ok".into(), - success: true, - tool_call_id: Some("tc-1".into()), + tool_call_id: Some("tc-3".into()), }]); - match msg { ConversationMessage::ToolResults(results) => { - assert_eq!(results.len(), 1); - assert_eq!(results[0].tool_call_id, "tc-1"); + assert_eq!(results[0].content, "access denied"); + assert!(!results[0].content.starts_with('\n')); } _ => panic!("expected ToolResults variant"), } } - // ── reasoning_content pass-through tests ────────────────────────────────── + // -- reasoning_content pass-through tests -- #[test] - fn native_to_provider_messages_includes_reasoning_content() { + fn to_provider_messages_native_includes_reasoning_content() { use super::super::history::to_provider_messages_native; + use crate::provider::traits::ToolCall; let history = vec![ConversationMessage::AssistantToolCalls { text: Some("answer".into()), tool_calls: vec![ToolCall { @@ -298,8 +183,9 @@ mod tests { } #[test] - fn native_to_provider_messages_omits_reasoning_content_when_none() { + fn to_provider_messages_native_omits_reasoning_content_when_none() { use super::super::history::to_provider_messages_native; + use crate::provider::traits::ToolCall; let history = vec![ConversationMessage::AssistantToolCalls { text: Some("answer".into()), tool_calls: vec![ToolCall { @@ -316,79 +202,4 @@ mod tests { let payload: serde_json::Value = serde_json::from_str(&messages[0].content).unwrap(); assert!(payload.get("reasoning_content").is_none()); } - - #[test] - fn xml_to_provider_messages_ignores_reasoning_content() { - use super::super::history::to_provider_messages_xml; - let history = vec![ConversationMessage::AssistantToolCalls { - text: Some("answer".into()), - tool_calls: vec![ToolCall { - id: "tc_1".into(), - name: "shell".into(), - arguments: "{}".into(), - }], - reasoning_content: Some("should be ignored".into()), - }]; - - let messages = to_provider_messages_xml(&history); - assert_eq!(messages.len(), 1); - assert_eq!(messages[0].role, "assistant"); - // XmlToolDispatcher returns text only, not JSON payload - assert_eq!(messages[0].content, "answer"); - assert!(!messages[0].content.contains("reasoning_content")); - } - - #[test] - fn xml_dispatcher_skips_empty_name() { - let response = ChatResponse { - text: Some( - "{\"name\":\"\",\"arguments\":{}}text after".into(), - ), - tool_calls: vec![], - usage: None, - reasoning_content: None, - }; - let dispatcher = XmlToolDispatcher; - let (text, calls) = dispatcher.parse_response(&response); - assert_eq!(calls.len(), 0); - assert!(text.contains("text after")); - } - - #[test] - fn xml_dispatcher_multiple_calls() { - let response = ChatResponse { - text: Some( - "intro\n{\"name\":\"a\",\"arguments\":{}}\n{\"name\":\"b\",\"arguments\":{\"x\":1}}".into(), - ), - tool_calls: vec![], - usage: None, - reasoning_content: None, - }; - let dispatcher = XmlToolDispatcher; - let (text, calls) = dispatcher.parse_response(&response); - assert_eq!(calls.len(), 2); - assert_eq!(calls[0].name, "a"); - assert_eq!(calls[1].name, "b"); - assert_eq!(calls[1].arguments["x"], 1); - assert!(text.contains("intro")); - } - - #[test] - fn native_dispatcher_bad_json_arguments_fallback() { - let response = ChatResponse { - text: None, - tool_calls: vec![ToolCall { - id: "tc2".into(), - name: "tool_x".into(), - arguments: "not-json".into(), - }], - usage: None, - reasoning_content: None, - }; - let dispatcher = NativeToolDispatcher; - let (_, calls) = dispatcher.parse_response(&response); - assert_eq!(calls.len(), 1); - assert!(calls[0].arguments.is_object()); - assert_eq!(calls[0].arguments, serde_json::json!({})); - } } diff --git a/crewforge-rs/src/agent/history.rs b/crewforge-rs/src/agent/history.rs index 239badc..946880b 100644 --- a/crewforge-rs/src/agent/history.rs +++ b/crewforge-rs/src/agent/history.rs @@ -1,6 +1,4 @@ -use crate::provider::traits::{ - ChatMessage, ConversationMessage, Provider, ToolResultMessage, -}; +use crate::provider::traits::{ChatMessage, ConversationMessage, Provider}; use anyhow::Result; use std::fmt::Write; @@ -49,34 +47,6 @@ pub fn to_provider_messages_native(history: &[ConversationMessage]) -> Vec Vec { - history - .iter() - .flat_map(|msg| match msg { - ConversationMessage::Chat(chat) => vec![chat.clone()], - ConversationMessage::AssistantToolCalls { text, .. } => { - vec![ChatMessage::assistant(text.clone().unwrap_or_default())] - } - ConversationMessage::ToolResults(results) => { - let mut content = String::new(); - for result in results { - let _ = writeln!( - content, - "\n{}\n", - result.tool_call_id, result.content - ); - } - vec![ChatMessage::user(format!("[Tool results]\n{content}"))] - } - }) - .collect() -} - /// Trim history to at most `max_messages` non-system ConversationMessages. /// /// Preserves the system prompt (first `Chat` with role=system) at index 0. @@ -110,7 +80,9 @@ fn build_compaction_transcript(messages: &[ConversationMessage]) -> String { let role = chat.role.to_uppercase(); let _ = writeln!(transcript, "{role}: {}", chat.content.trim()); } - ConversationMessage::AssistantToolCalls { text, tool_calls, .. } => { + ConversationMessage::AssistantToolCalls { + text, tool_calls, .. + } => { let text_str = text.as_deref().unwrap_or("").trim(); let _ = writeln!(transcript, "ASSISTANT: {text_str}"); for tc in tool_calls { @@ -258,12 +230,10 @@ mod tests { #[test] fn to_provider_messages_native_tool_results() { - let history = vec![ConversationMessage::ToolResults(vec![ - ToolResultMessage { - tool_call_id: "tc1".into(), - content: "output".into(), - }, - ])]; + let history = vec![ConversationMessage::ToolResults(vec![ToolResultMessage { + tool_call_id: "tc1".into(), + content: "output".into(), + }])]; let msgs = to_provider_messages_native(&history); assert_eq!(msgs.len(), 1); assert_eq!(msgs[0].role, "tool"); @@ -272,38 +242,6 @@ mod tests { assert_eq!(payload["content"].as_str(), Some("output")); } - #[test] - fn to_provider_messages_xml_assistant_tool_calls_text_only() { - let history = vec![ConversationMessage::AssistantToolCalls { - text: Some("answer".into()), - tool_calls: vec![ToolCall { - id: "tc1".into(), - name: "shell".into(), - arguments: "{}".into(), - }], - reasoning_content: Some("ignored".into()), - }]; - let msgs = to_provider_messages_xml(&history); - assert_eq!(msgs.len(), 1); - assert_eq!(msgs[0].role, "assistant"); - assert_eq!(msgs[0].content, "answer"); - } - - #[test] - fn to_provider_messages_xml_tool_results_as_user() { - let history = vec![ConversationMessage::ToolResults(vec![ - ToolResultMessage { - tool_call_id: "tc1".into(), - content: "result_data".into(), - }, - ])]; - let msgs = to_provider_messages_xml(&history); - assert_eq!(msgs.len(), 1); - assert_eq!(msgs[0].role, "user"); - assert!(msgs[0].content.contains("[Tool results]")); - assert!(msgs[0].content.contains("result_data")); - } - #[test] fn trim_history_preserves_system_and_recent() { let mut history = vec![ @@ -318,9 +256,7 @@ mod tests { // system + 3 most recent non-system assert_eq!(history.len(), 4); // system is still first - assert!( - matches!(&history[0], ConversationMessage::Chat(m) if m.role == "system") - ); + assert!(matches!(&history[0], ConversationMessage::Chat(m) if m.role == "system")); // most recent messages are preserved assert!( matches!(&history[history.len()-1], ConversationMessage::Chat(m) if m.content == "msg3") diff --git a/crewforge-rs/src/agent/loop_.rs b/crewforge-rs/src/agent/loop_.rs index 61466f6..3d491ec 100644 --- a/crewforge-rs/src/agent/loop_.rs +++ b/crewforge-rs/src/agent/loop_.rs @@ -5,22 +5,18 @@ use std::collections::HashSet; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; -use crate::provider::traits::{ - ChatMessage, ChatRequest, ConversationMessage, Provider, ToolCall, ToolSpec, -}; use super::Tool; -use super::dispatcher::{ - NativeToolDispatcher, ParsedToolCall, ToolDispatcher, ToolExecutionResult, XmlToolDispatcher, -}; -use super::history::{ - auto_compact_history, to_provider_messages_native, to_provider_messages_xml, trim_history, -}; +use super::dispatcher::{self, ParsedToolCall, ToolExecutionResult}; +use super::history::{auto_compact_history, to_provider_messages_native, trim_history}; +use crate::provider::traits::{ChatMessage, ChatRequest, ConversationMessage, Provider, ToolSpec}; // ── Public event/config/stop types ─────────────────────────────────────────── #[derive(Debug, Clone)] pub enum AgentEvent { - LlmThinking { iteration: usize }, + LlmThinking { + iteration: usize, + }, LlmResponse { text: Option, tool_call_count: usize, @@ -93,7 +89,9 @@ impl AgentSession { let system_prompt = system_prompt.into(); let mut history = Vec::new(); if !system_prompt.is_empty() { - history.push(ConversationMessage::Chat(ChatMessage::system(system_prompt))); + history.push(ConversationMessage::Chat(ChatMessage::system( + system_prompt, + ))); } Self { provider, @@ -126,7 +124,9 @@ impl AgentSession { // Add initial user message to history. self.history - .push(ConversationMessage::Chat(ChatMessage::user(initial_message))); + .push(ConversationMessage::Chat(ChatMessage::user( + initial_message, + ))); // Compact and trim history before starting. let _ = auto_compact_history( @@ -138,13 +138,12 @@ impl AgentSession { .await; trim_history(&mut self.history, self.config.max_history_messages); - let use_native = self.provider.supports_native_tools() && !self.tools.is_empty(); let tool_specs: Vec = self.tools.iter().map(|t| t.spec()).collect(); let mut seen_signatures: HashSet<(String, String)> = HashSet::new(); let mut final_text: Option = None; let mut iterations_used = 0; - 'outer: for iteration in 0..self.config.max_iterations { + for iteration in 0..self.config.max_iterations { if self.cancelled.load(Ordering::SeqCst) { events.push(AgentEvent::TurnFinished { final_text, @@ -158,30 +157,21 @@ impl AgentSession { iterations_used = iteration + 1; // Build provider messages from history. - let messages = if use_native { - to_provider_messages_native(&self.history) - } else { - to_provider_messages_xml(&self.history) - }; + let messages = to_provider_messages_native(&self.history); // Call the provider. - let response = if use_native && !tool_specs.is_empty() { - let request = ChatRequest { - messages: &messages, - tools: Some(&tool_specs), - }; - self.provider - .chat(request, &self.model, self.config.temperature) - .await - } else { - let request = ChatRequest { - messages: &messages, - tools: None, - }; - self.provider - .chat(request, &self.model, self.config.temperature) - .await + let request = ChatRequest { + messages: &messages, + tools: if tool_specs.is_empty() { + None + } else { + Some(&tool_specs) + }, }; + let response = self + .provider + .chat(request, &self.model, self.config.temperature) + .await; let response = match response { Ok(r) => r, @@ -200,11 +190,7 @@ impl AgentSession { }; // Parse tool calls from the response. - let (text, parsed_calls) = if use_native { - NativeToolDispatcher.parse_response(&response) - } else { - XmlToolDispatcher.parse_response(&response) - }; + let (text, parsed_calls) = dispatcher::parse_response(&response); let usage = response.usage.clone(); @@ -221,7 +207,9 @@ impl AgentSession { if parsed_calls.is_empty() { // No tool calls — this is the final response for this turn. self.history - .push(ConversationMessage::Chat(ChatMessage::assistant(text.clone()))); + .push(ConversationMessage::Chat(ChatMessage::assistant( + text.clone(), + ))); final_text = Some(text); events.push(AgentEvent::TurnFinished { final_text: final_text.clone(), @@ -232,20 +220,13 @@ impl AgentSession { } // Store the assistant's tool-call turn in history. - let tool_calls_for_history: Vec = if use_native { - response.tool_calls.clone() - } else { - parsed_calls - .iter() - .map(|pc| ToolCall { - id: uuid::Uuid::new_v4().to_string(), - name: pc.name.clone(), - arguments: pc.arguments.to_string(), - }) - .collect() - }; + let tool_calls_for_history = response.tool_calls.clone(); self.history.push(ConversationMessage::AssistantToolCalls { - text: if text.is_empty() { None } else { Some(text.clone()) }, + text: if text.is_empty() { + None + } else { + Some(text.clone()) + }, tool_calls: tool_calls_for_history, reasoning_content: response.reasoning_content.clone(), }); @@ -279,8 +260,12 @@ impl AgentSession { }); let result = execute_tool(&self.tools, call).await; - let success = result.success; - let output = result.output.clone(); + let success = result.tool_result.success; + let output = if let Some(ref err) = result.tool_result.error { + err.clone() + } else { + result.tool_result.output.clone() + }; events.push(AgentEvent::ToolCallFinished { name: call.name.clone(), @@ -292,11 +277,7 @@ impl AgentSession { } // Store tool results in history. - let history_entry = if use_native { - NativeToolDispatcher.format_results(&tool_results) - } else { - XmlToolDispatcher.format_results(&tool_results) - }; + let history_entry = dispatcher::format_results(&tool_results); self.history.push(history_entry); // Check cancellation after each tool batch. @@ -308,7 +289,6 @@ impl AgentSession { }); return events; } - } // Max iterations reached. @@ -333,24 +313,29 @@ fn tool_call_signature(name: &str, arguments: &serde_json::Value) -> (String, St async fn execute_tool(tools: &[Box], call: &ParsedToolCall) -> ToolExecutionResult { let tool = tools.iter().find(|t| t.name() == call.name); match tool { - Some(t) => match t.call(call.arguments.clone()).await { - Ok(output) => ToolExecutionResult { + Some(t) => match t.execute(call.arguments.clone()).await { + Ok(tool_result) => ToolExecutionResult { name: call.name.clone(), - output, - success: true, + tool_result, tool_call_id: call.tool_call_id.clone(), }, Err(e) => ToolExecutionResult { name: call.name.clone(), - output: format!("Error: {e}"), - success: false, + tool_result: super::ToolResult { + success: false, + output: String::new(), + error: Some(format!("Error: {e}")), + }, tool_call_id: call.tool_call_id.clone(), }, }, None => ToolExecutionResult { name: call.name.clone(), - output: format!("Unknown tool: {}", call.name), - success: false, + tool_result: super::ToolResult { + success: false, + output: String::new(), + error: Some(format!("Unknown tool: {}", call.name)), + }, tool_call_id: call.tool_call_id.clone(), }, } @@ -361,8 +346,8 @@ async fn execute_tool(tools: &[Box], call: &ParsedToolCall) -> ToolExe /// Matches patterns such as `token=...`, `api_key: "..."`, `password=...`, etc. /// Preserves the first 4 characters of each value for context; redacts the rest. pub fn scrub_credentials(input: &str) -> String { - use std::sync::LazyLock; use regex::Regex; + use std::sync::LazyLock; // Matches key=value and key: value forms (token=, api_key:, bearer:, etc.) static SENSITIVE_KV_REGEX: LazyLock = LazyLock::new(|| { @@ -518,7 +503,11 @@ mod tests { ))); // Should have LlmThinking at start. - assert!(events.iter().any(|e| matches!(e, AgentEvent::LlmThinking { iteration: 0 }))); + assert!( + events + .iter() + .any(|e| matches!(e, AgentEvent::LlmThinking { iteration: 0 })) + ); // History should include system, user, and assistant messages. assert!(session.history.len() >= 3); @@ -559,17 +548,21 @@ mod tests { let events = session.run_turn("anything").await; - // First turn is consumed (provider called once before cancel check kicks in - // inside the loop), OR cancelled before LLM call — depends on loop ordering. - // Either way we must have a TurnFinished with Cancelled or Done. - let finished = events.iter().find(|e| matches!(e, AgentEvent::TurnFinished { .. })); - assert!(finished.is_some()); + // Cancellation check is the first statement in the loop body, + // so a pre-cancelled session always produces StopReason::Cancelled. + assert!(events.iter().any(|e| matches!( + e, + AgentEvent::TurnFinished { + stop_reason: StopReason::Cancelled, + .. + } + ))); } #[tokio::test] async fn run_turn_cancel_handle_signals_session() { let provider = Arc::new(EchoProvider); - let mut session = AgentSession::new( + let session = AgentSession::new( provider, "test-model", "", @@ -585,7 +578,7 @@ mod tests { #[tokio::test] async fn run_turn_max_iterations() { - // Provider that always returns a tool call so iterations keep running. + // Provider that always returns a native tool call so iterations keep running. struct LoopProvider; #[async_trait] @@ -597,10 +590,25 @@ mod tests { _model: &str, _temperature: f64, ) -> anyhow::Result { - // Return XML tool call that will be unique each time (different arg value - // ensures dedup doesn't shortcircuit first). - // Actually for max-iter test we want dedup to NOT kill it — use static call. - Ok("{\"name\":\"noop\",\"arguments\":{}}".to_string()) + Ok(String::new()) + } + + async fn chat( + &self, + _request: crate::provider::traits::ChatRequest<'_>, + _model: &str, + _temperature: f64, + ) -> anyhow::Result { + Ok(crate::provider::traits::ChatResponse { + text: None, + tool_calls: vec![crate::provider::traits::ToolCall { + id: uuid::Uuid::new_v4().to_string(), + name: "noop".into(), + arguments: "{}".into(), + }], + usage: None, + reasoning_content: None, + }) } } @@ -609,11 +617,24 @@ mod tests { #[async_trait] impl Tool for NoopTool { - fn name(&self) -> &str { "noop" } - fn description(&self) -> &str { "no-op" } - fn parameters(&self) -> serde_json::Value { serde_json::json!({}) } - async fn call(&self, _args: serde_json::Value) -> anyhow::Result { - Ok("done".to_string()) + fn name(&self) -> &str { + "noop" + } + fn description(&self) -> &str { + "no-op" + } + fn parameters(&self) -> serde_json::Value { + serde_json::json!({}) + } + async fn execute( + &self, + _args: serde_json::Value, + ) -> anyhow::Result { + Ok(crate::agent::ToolResult { + success: true, + output: "done".to_string(), + error: None, + }) } } diff --git a/crewforge-rs/src/agent/mod.rs b/crewforge-rs/src/agent/mod.rs index 8b39b07..2eddb38 100644 --- a/crewforge-rs/src/agent/mod.rs +++ b/crewforge-rs/src/agent/mod.rs @@ -1,12 +1,39 @@ pub mod dispatcher; pub mod history; pub mod loop_; -pub mod prompt; pub use loop_::{AgentEvent, AgentSession, AgentSessionConfig, StopReason}; -use async_trait::async_trait; use crate::provider::traits::ToolSpec; +use async_trait::async_trait; + +/// Structured result from tool execution. +/// Security denials use `success: false` with an `error` message — +/// they are business logic, not program errors. +#[derive(Debug, Clone)] +pub struct ToolResult { + pub success: bool, + pub output: String, + pub error: Option, +} + +impl ToolResult { + pub fn ok(output: impl Into) -> Self { + Self { + success: true, + output: output.into(), + error: None, + } + } + + pub fn denied(message: impl Into) -> Self { + Self { + success: false, + output: String::new(), + error: Some(message.into()), + } + } +} /// Generic tool interface. Implement this for any tool the agent can use. /// CrewForge hub tools (HubGet, HubAck, HubPost) implement this trait. @@ -24,5 +51,11 @@ pub trait Tool: Send + Sync { } } - async fn call(&self, args: serde_json::Value) -> anyhow::Result; + /// Whether this tool has side effects (writes files, runs commands, etc.). + /// Non-mutating tools (reads, searches) can be auto-approved and parallelized. + fn is_mutating(&self) -> bool { + false // safe default: assume read-only + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result; } diff --git a/crewforge-rs/src/agent/prompt.rs b/crewforge-rs/src/agent/prompt.rs deleted file mode 100644 index 69cf5f7..0000000 --- a/crewforge-rs/src/agent/prompt.rs +++ /dev/null @@ -1,38 +0,0 @@ -/// Build a basic system prompt for an agent with optional instructions. -pub fn build_system_prompt( - agent_name: &str, - instructions: Option<&str>, -) -> String { - let mut prompt = format!("You are {agent_name}, an AI assistant."); - if let Some(instr) = instructions { - if !instr.is_empty() { - prompt.push_str("\n\n"); - prompt.push_str(instr); - } - } - prompt -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn build_system_prompt_no_instructions() { - let prompt = build_system_prompt("CrewForgeAgent", None); - assert_eq!(prompt, "You are CrewForgeAgent, an AI assistant."); - } - - #[test] - fn build_system_prompt_with_instructions() { - let prompt = build_system_prompt("CrewForgeAgent", Some("Be concise.")); - assert!(prompt.contains("CrewForgeAgent")); - assert!(prompt.contains("Be concise.")); - } - - #[test] - fn build_system_prompt_empty_instructions() { - let prompt = build_system_prompt("CrewForgeAgent", Some("")); - assert_eq!(prompt, "You are CrewForgeAgent, an AI assistant."); - } -} diff --git a/crewforge-rs/src/agent_cmd.rs b/crewforge-rs/src/agent_cmd.rs index c15e0a0..70564e4 100644 --- a/crewforge-rs/src/agent_cmd.rs +++ b/crewforge-rs/src/agent_cmd.rs @@ -14,12 +14,13 @@ use std::io::{self, BufRead, Write}; use std::sync::Arc; use anyhow::Result; -use async_trait::async_trait; use clap::Args; use crewforge::{ agent::{AgentEvent, AgentSession, AgentSessionConfig, StopReason, Tool}, auth::{AuthService, default_state_dir}, provider::{self, default_api_key_env}, + security::SecurityPolicy, + tools::{TokioRuntime, default_tools}, }; // ── Clap args ───────────────────────────────────────────────────────────────── @@ -59,54 +60,6 @@ pub struct AgentArgs { temperature: f64, } -// ── Built-in test tools ─────────────────────────────────────────────────────── - -struct EchoTool; - -#[async_trait] -impl Tool for EchoTool { - fn name(&self) -> &str { "echo" } - fn description(&self) -> &str { - "Echo back the provided message. Useful for verifying that tool calling works end-to-end." - } - fn parameters(&self) -> serde_json::Value { - serde_json::json!({ - "type": "object", - "properties": { - "message": {"type": "string", "description": "The message to echo back"} - }, - "required": ["message"] - }) - } - async fn call(&self, args: serde_json::Value) -> anyhow::Result { - let msg = args.get("message").and_then(|v| v.as_str()).unwrap_or("[no message]"); - Ok(format!("Echo: {msg}")) - } -} - -struct DatetimeTool; - -#[async_trait] -impl Tool for DatetimeTool { - fn name(&self) -> &str { "get_datetime" } - fn description(&self) -> &str { "Get the current UTC date and time." } - fn parameters(&self) -> serde_json::Value { - serde_json::json!({"type": "object", "properties": {}, "required": []}) - } - async fn call(&self, _args: serde_json::Value) -> anyhow::Result { - use std::time::{SystemTime, UNIX_EPOCH}; - let secs = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0); - let s = secs % 60; - let m = (secs / 60) % 60; - let h = (secs / 3600) % 24; - let days = secs / 86400; - Ok(format!("UTC unix_day={days} {:02}:{:02}:{:02}", h, m, s)) - } -} - // ── Event rendering ─────────────────────────────────────────────────────────── fn print_event(event: &AgentEvent) { @@ -118,35 +71,56 @@ fn print_event(event: &AgentEvent) { eprintln!("\x1b[2m[thinking... round {}]\x1b[0m", iteration + 1); } } - AgentEvent::LlmResponse { text, tool_call_count, usage } => { + AgentEvent::LlmResponse { + text, + tool_call_count, + usage, + } => { if *tool_call_count == 0 { if let Some(t) = text { println!("{t}"); } - } else if let Some(t) = text { - if !t.is_empty() { - eprintln!("\x1b[2m[llm]: {t}\x1b[0m"); - } + } else if let Some(t) = text + && !t.is_empty() + { + eprintln!("\x1b[2m[llm]: {t}\x1b[0m"); } - if let Some(u) = usage { - if u.input_tokens.is_some() || u.output_tokens.is_some() { - eprintln!( - "\x1b[2m[tokens] in={} out={}\x1b[0m", - u.input_tokens.unwrap_or(0), - u.output_tokens.unwrap_or(0) - ); - } + if let Some(u) = usage + && (u.input_tokens.is_some() || u.output_tokens.is_some()) + { + eprintln!( + "\x1b[2m[tokens] in={} out={}\x1b[0m", + u.input_tokens.unwrap_or(0), + u.output_tokens.unwrap_or(0) + ); } } - AgentEvent::ToolCallStarted { iteration, name, args } => { + AgentEvent::ToolCallStarted { + iteration, + name, + args, + } => { let args_str = serde_json::to_string(args).unwrap_or_else(|_| "{}".to_string()); - eprintln!("\x1b[33m[tool:{}] {} {}\x1b[0m", iteration + 1, name, args_str); + eprintln!( + "\x1b[33m[tool:{}] {} {}\x1b[0m", + iteration + 1, + name, + args_str + ); } - AgentEvent::ToolCallFinished { name, result, success } => { + AgentEvent::ToolCallFinished { + name, + result, + success, + } => { let icon = if *success { "✓" } else { "✗" }; eprintln!("\x1b[32m[{icon} {name}] {result}\x1b[0m"); } - AgentEvent::TurnFinished { final_text, iterations_used, stop_reason } => { + AgentEvent::TurnFinished { + final_text, + iterations_used, + stop_reason, + } => { let reason = match stop_reason { StopReason::Done => "done", StopReason::MaxIterations => "max_iterations", @@ -156,10 +130,10 @@ fn print_event(event: &AgentEvent) { "\x1b[2m[turn finished: {} iteration(s), reason={}]\x1b[0m", iterations_used, reason ); - if *iterations_used == 0 { - if let Some(t) = final_text { - println!("{t}"); - } + if *iterations_used == 0 + && let Some(t) = final_text + { + println!("{t}"); } } AgentEvent::Error { message, fatal } => { @@ -197,7 +171,13 @@ pub async fn run(args: AgentArgs) -> Result<()> { let tools: Vec> = if args.no_tools { vec![] } else { - vec![Box::new(EchoTool), Box::new(DatetimeTool)] + let workspace = std::env::current_dir().unwrap_or_else(|_| ".".into()); + let security = Arc::new(SecurityPolicy { + workspace_dir: workspace, + ..SecurityPolicy::default() + }); + let runtime = Arc::new(TokioRuntime); + default_tools(security, runtime) }; let config = AgentSessionConfig { @@ -206,17 +186,19 @@ pub async fn run(args: AgentArgs) -> Result<()> { ..Default::default() }; + let tool_names: Vec = tools.iter().map(|t| t.name().to_string()).collect(); let mut session = AgentSession::new(provider, &args.model, &args.system, tools, config); eprintln!( "\x1b[1mcrewforge agent\x1b[0m provider={} model={} tools={}", args.provider, args.model, - if args.no_tools { "off" } else { "echo,get_datetime" } + if args.no_tools { + "off".to_string() + } else { + tool_names.join(", ") + } ); - if !args.no_tools { - eprintln!("tools: echo(message), get_datetime()"); - } eprintln!("Type your message and press Enter. Ctrl-D to exit.\n"); let stdin = io::stdin(); diff --git a/crewforge-rs/src/auth/gemini_oauth.rs b/crewforge-rs/src/auth/gemini_oauth.rs index 3deb7a2..e3f6c91 100644 --- a/crewforge-rs/src/auth/gemini_oauth.rs +++ b/crewforge-rs/src/auth/gemini_oauth.rs @@ -21,7 +21,7 @@ use tokio::net::TcpListener; // Re-export for external use #[allow(unused_imports)] -pub use crate::auth::oauth_common::{generate_pkce_state, PkceState}; +pub use crate::auth::oauth_common::{PkceState, generate_pkce_state}; /// Get Gemini OAuth client ID from environment. /// Required: set GEMINI_OAUTH_CLIENT_ID environment variable. @@ -478,12 +478,11 @@ pub fn parse_code_from_redirect(input: &str, expected_state: Option<&str>) -> Re let params = parse_query_params(query); if let Some(code) = params.get("code") { - if let Some(expected) = expected_state { - if let Some(actual) = params.get("state") { - if actual != expected { - anyhow::bail!("OAuth state mismatch: expected {expected}, got {actual}"); - } - } + if let Some(expected) = expected_state + && let Some(actual) = params.get("state") + && actual != expected + { + anyhow::bail!("OAuth state mismatch: expected {expected}, got {actual}"); } return Ok(code.clone()); } diff --git a/crewforge-rs/src/auth/mod.rs b/crewforge-rs/src/auth/mod.rs index cbb466a..303fc96 100644 --- a/crewforge-rs/src/auth/mod.rs +++ b/crewforge-rs/src/auth/mod.rs @@ -6,7 +6,7 @@ pub mod profiles; use crate::auth::openai_oauth::refresh_access_token; use crate::auth::profiles::{ - profile_id, AuthProfile, AuthProfileKind, AuthProfilesData, AuthProfilesStore, TokenSet, + AuthProfile, AuthProfileKind, AuthProfilesData, AuthProfilesStore, TokenSet, profile_id, }; use anyhow::Result; use std::collections::HashMap; @@ -384,10 +384,10 @@ pub fn select_profile_id( return None; } - if let Some(active) = data.active_profiles.get(provider) { - if data.profiles.contains_key(active) { - return Some(active.clone()); - } + if let Some(active) = data.active_profiles.get(provider) + && data.profiles.contains_key(active) + { + return Some(active.clone()); } let default = default_profile_id(provider); diff --git a/crewforge-rs/src/auth/oauth_common.rs b/crewforge-rs/src/auth/oauth_common.rs index fc499d0..caa5ed1 100644 --- a/crewforge-rs/src/auth/oauth_common.rs +++ b/crewforge-rs/src/auth/oauth_common.rs @@ -35,7 +35,7 @@ pub fn generate_pkce_state() -> PkceState { /// Generate a cryptographically random base64url-encoded string. pub fn random_base64url(byte_len: usize) -> String { - use chacha20poly1305::aead::{rand_core::RngCore, OsRng}; + use chacha20poly1305::aead::{OsRng, rand_core::RngCore}; let mut bytes = vec![0_u8; byte_len]; OsRng.fill_bytes(&mut bytes); @@ -66,12 +66,12 @@ pub fn url_decode(input: &str) -> String { b'%' if i + 2 < bytes.len() => { let hi = bytes[i + 1] as char; let lo = bytes[i + 2] as char; - if let (Some(h), Some(l)) = (hi.to_digit(16), lo.to_digit(16)) { - if let Ok(value) = u8::try_from(h * 16 + l) { - out.push(value); - i += 3; - continue; - } + if let (Some(h), Some(l)) = (hi.to_digit(16), lo.to_digit(16)) + && let Ok(value) = u8::try_from(h * 16 + l) + { + out.push(value); + i += 3; + continue; } out.push(bytes[i]); i += 1; diff --git a/crewforge-rs/src/auth/openai_oauth.rs b/crewforge-rs/src/auth/openai_oauth.rs index 218ff11..4c84d9e 100644 --- a/crewforge-rs/src/auth/openai_oauth.rs +++ b/crewforge-rs/src/auth/openai_oauth.rs @@ -13,7 +13,7 @@ use tokio::net::TcpListener; // Re-export for external use #[allow(unused_imports)] -pub use crate::auth::oauth_common::{generate_pkce_state, PkceState}; +pub use crate::auth::oauth_common::{PkceState, generate_pkce_state}; pub const OPENAI_OAUTH_CLIENT_ID: &str = "app_EMoamEEZ73f0CkXaXp7hrann"; pub const OPENAI_OAUTH_AUTHORIZE_URL: &str = "https://auth.openai.com/oauth/authorize"; @@ -328,10 +328,10 @@ pub fn extract_account_id_from_jwt(token: &str) -> Option { "sub", "https://api.openai.com/account_id", ] { - if let Some(value) = claims.get(key).and_then(|v| v.as_str()) { - if !value.trim().is_empty() { - return Some(value.to_string()); - } + if let Some(value) = claims.get(key).and_then(|v| v.as_str()) + && !value.trim().is_empty() + { + return Some(value.to_string()); } } @@ -409,9 +409,10 @@ mod tests { Some("xyz"), ) .unwrap_err(); - assert!(err - .to_string() - .contains("OpenAI OAuth error: access_denied")); + assert!( + err.to_string() + .contains("OpenAI OAuth error: access_denied") + ); } #[test] diff --git a/crewforge-rs/src/auth_cmd.rs b/crewforge-rs/src/auth_cmd.rs index 69fbe83..75ba0f6 100644 --- a/crewforge-rs/src/auth_cmd.rs +++ b/crewforge-rs/src/auth_cmd.rs @@ -1,11 +1,11 @@ //! `crewforge auth` subcommand — OAuth login, token management, and profile listing. -use crewforge::auth::{self, AuthService, default_state_dir, normalize_provider}; -use crewforge::auth::oauth_common::PkceState; -use crewforge::security::SecretStore; -use anyhow::{bail, Result}; +use anyhow::{Result, bail}; use chrono::{DateTime, Utc}; use clap::Subcommand; +use crewforge::auth::oauth_common::PkceState; +use crewforge::auth::{self, AuthService, default_state_dir, normalize_provider}; +use crewforge::security::SecretStore; use serde::{Deserialize, Serialize}; use std::path::PathBuf; @@ -341,9 +341,7 @@ async fn run_login(provider: String, profile: String, device_code: bool) -> Resu match provider.as_str() { "gemini" => run_gemini_login(&svc, &client, &profile, device_code).await, "openai-codex" => run_openai_login(&svc, &client, &profile, device_code).await, - _ => bail!( - "`auth login` supports --provider openai-codex or gemini, got: {provider}" - ), + _ => bail!("`auth login` supports --provider openai-codex or gemini, got: {provider}"), } } @@ -407,15 +405,12 @@ async fn run_gemini_login( } Err(e) => { println!("Callback capture failed: {e}"); - println!( - "Run `crewforge auth paste-redirect --provider gemini --profile {profile}`" - ); + println!("Run `crewforge auth paste-redirect --provider gemini --profile {profile}`"); return Ok(()); } }; - let token_set = - auth::gemini_oauth::exchange_code_for_tokens(client, &code, &pkce).await?; + let token_set = auth::gemini_oauth::exchange_code_for_tokens(client, &code, &pkce).await?; let account_id = token_set .id_token .as_deref() @@ -456,9 +451,7 @@ async fn run_openai_login( return Ok(()); } Err(e) => { - println!( - "Device-code flow unavailable: {e}. Falling back to browser/paste flow." - ); + println!("Device-code flow unavailable: {e}. Falling back to browser/paste flow."); } } } @@ -494,8 +487,7 @@ async fn run_openai_login( } }; - let token_set = - auth::openai_oauth::exchange_code_for_tokens(client, &code, &pkce).await?; + let token_set = auth::openai_oauth::exchange_code_for_tokens(client, &code, &pkce).await?; let account_id = extract_openai_account_id(&token_set.access_token); svc.store_openai_tokens(profile, token_set, account_id, true) .await?; @@ -613,10 +605,12 @@ async fn run_paste_token( if token.is_empty() { bail!("Token cannot be empty"); } - let kind = - auth::anthropic_token::detect_auth_kind(&token, auth_kind.as_deref()); + let kind = auth::anthropic_token::detect_auth_kind(&token, auth_kind.as_deref()); let mut metadata = std::collections::HashMap::new(); - metadata.insert("auth_kind".to_string(), kind.as_metadata_value().to_string()); + metadata.insert( + "auth_kind".to_string(), + kind.as_metadata_value().to_string(), + ); let svc = make_auth_service(); svc.store_provider_token(&provider, &profile, &token, metadata, true) @@ -638,9 +632,7 @@ async fn run_refresh(provider: String, profile: Option) -> Result<()> { .get_valid_openai_access_token(profile.as_deref()) .await? { - Some(_) => println!( - "OpenAI Codex token is valid (refresh completed if needed)." - ), + Some(_) => println!("OpenAI Codex token is valid (refresh completed if needed)."), None => bail!( "No OpenAI Codex auth profile found. \ Run `crewforge auth login --provider openai-codex`." diff --git a/crewforge-rs/src/bin/agentctl.rs b/crewforge-rs/src/bin/agentctl.rs index d55bc35..e10ad36 100644 --- a/crewforge-rs/src/bin/agentctl.rs +++ b/crewforge-rs/src/bin/agentctl.rs @@ -16,7 +16,7 @@ use async_trait::async_trait; use clap::Parser; use crewforge::{ agent::{AgentEvent, AgentSession, AgentSessionConfig, StopReason, Tool}, - provider::{self, ToolSpec}, + provider::{self}, }; // ── CLI args ────────────────────────────────────────────────────────────────── @@ -88,12 +88,19 @@ impl Tool for EchoTool { }) } - async fn call(&self, args: serde_json::Value) -> anyhow::Result { + async fn execute( + &self, + args: serde_json::Value, + ) -> anyhow::Result { let msg = args .get("message") .and_then(|v| v.as_str()) .unwrap_or("[no message]"); - Ok(format!("Echo: {msg}")) + Ok(crewforge::agent::ToolResult { + success: true, + output: format!("Echo: {msg}"), + error: None, + }) } } @@ -118,18 +125,24 @@ impl Tool for DatetimeTool { }) } - async fn call(&self, _args: serde_json::Value) -> anyhow::Result { + async fn execute( + &self, + _args: serde_json::Value, + ) -> anyhow::Result { use std::time::{SystemTime, UNIX_EPOCH}; let secs = SystemTime::now() .duration_since(UNIX_EPOCH) .map(|d| d.as_secs()) .unwrap_or(0); - // Simple ISO-like format without chrono let s = secs % 60; let m = (secs / 60) % 60; let h = (secs / 3600) % 24; let days = secs / 86400; - Ok(format!("UTC unix_day={days} {:02}:{:02}:{:02}", h, m, s)) + Ok(crewforge::agent::ToolResult { + success: true, + output: format!("UTC unix_day={days} {:02}:{:02}:{:02}", h, m, s), + error: None, + }) } } @@ -153,21 +166,19 @@ fn print_event(event: &AgentEvent) { if let Some(t) = text { println!("{t}"); } - } else { - if let Some(t) = text { - if !t.is_empty() { - eprintln!("\x1b[2m[llm]: {t}\x1b[0m"); - } - } + } else if let Some(t) = text + && !t.is_empty() + { + eprintln!("\x1b[2m[llm]: {t}\x1b[0m"); } - if let Some(u) = usage { - if u.input_tokens.is_some() || u.output_tokens.is_some() { - eprintln!( - "\x1b[2m[tokens] in={} out={}\x1b[0m", - u.input_tokens.unwrap_or(0), - u.output_tokens.unwrap_or(0) - ); - } + if let Some(u) = usage + && (u.input_tokens.is_some() || u.output_tokens.is_some()) + { + eprintln!( + "\x1b[2m[tokens] in={} out={}\x1b[0m", + u.input_tokens.unwrap_or(0), + u.output_tokens.unwrap_or(0) + ); } } AgentEvent::ToolCallStarted { @@ -206,10 +217,10 @@ fn print_event(event: &AgentEvent) { iterations_used, reason ); // final_text already printed via LlmResponse when tool_call_count==0 - if *iterations_used == 0 { - if let Some(t) = final_text { - println!("{t}"); - } + if *iterations_used == 0 + && let Some(t) = final_text + { + println!("{t}"); } } AgentEvent::Error { message, fatal } => { @@ -236,10 +247,7 @@ async fn main() -> anyhow::Result<()> { None // create_provider will pick it up from the env var } else { // Fall back to stored auth profile. - let svc = crewforge::auth::AuthService::new( - &crewforge::auth::default_state_dir(), - false, - ); + let svc = crewforge::auth::AuthService::new(&crewforge::auth::default_state_dir(), false); svc.get_provider_bearer_token(&args.provider, None) .await .unwrap_or(None) @@ -265,13 +273,7 @@ async fn main() -> anyhow::Result<()> { ..Default::default() }; - let mut session = AgentSession::new( - provider, - &args.model, - &args.system, - tools, - config, - ); + let mut session = AgentSession::new(provider, &args.model, &args.system, tools, config); // Migration notice. eprintln!("\x1b[2m[note] agentctl is an internal tool. Use `crewforge agent` instead.\x1b[0m"); @@ -281,7 +283,11 @@ async fn main() -> anyhow::Result<()> { "\x1b[1magentctl\x1b[0m provider={} model={} tools={}", args.provider, args.model, - if args.no_tools { "off" } else { "echo,get_datetime" } + if args.no_tools { + "off" + } else { + "echo,get_datetime" + } ); if !args.no_tools { eprintln!("tools: echo(message), get_datetime()"); diff --git a/crewforge-rs/src/chat.rs b/crewforge-rs/src/chat.rs index 5c39875..a46c928 100644 --- a/crewforge-rs/src/chat.rs +++ b/crewforge-rs/src/chat.rs @@ -21,9 +21,9 @@ use crate::hub::{RateLimitUsage, RoomHub}; use crate::kernel::{MessageEvent, MessageRole, SessionKernel}; use crate::managed_opencode::{self, HUB_ACK_TOOL, HUB_GET_TOOL, HUB_POST_TOOL}; use crate::mcp_server::RoomHubMcpServer; +use crate::opencode_provider::{OpencodeCliProvider, OpencodeProviderConfig}; use crate::profiles::{self, GlobalProfile}; use crate::prompt_theme; -use crate::opencode_provider::{OpencodeCliProvider, OpencodeProviderConfig}; use crate::scheduler::{WakeDecision, WorkerState, decide_wake, on_wake_finished}; use crate::text::{format_time, to_single_line_error}; use crate::tui::DisplayLine; diff --git a/crewforge-rs/src/lib.rs b/crewforge-rs/src/lib.rs index a6ac1b0..6729c88 100644 --- a/crewforge-rs/src/lib.rs +++ b/crewforge-rs/src/lib.rs @@ -2,3 +2,4 @@ pub mod agent; pub mod auth; pub mod provider; pub mod security; +pub mod tools; diff --git a/crewforge-rs/src/main.rs b/crewforge-rs/src/main.rs index 9b84f3f..a75917c 100644 --- a/crewforge-rs/src/main.rs +++ b/crewforge-rs/src/main.rs @@ -7,9 +7,9 @@ mod init; mod kernel; mod managed_opencode; mod mcp_server; +mod opencode_provider; mod profiles; mod prompt_theme; -mod opencode_provider; mod scheduler; mod text; mod tui; diff --git a/crewforge-rs/src/provider/anthropic.rs b/crewforge-rs/src/provider/anthropic.rs index 74453f6..0224794 100644 --- a/crewforge-rs/src/provider/anthropic.rs +++ b/crewforge-rs/src/provider/anthropic.rs @@ -38,10 +38,13 @@ struct ContentBlock { kind: String, #[serde(default)] text: Option, + #[allow(dead_code)] #[serde(default)] id: Option, + #[allow(dead_code)] #[serde(default)] name: Option, + #[allow(dead_code)] #[serde(default)] input: Option, } @@ -208,15 +211,15 @@ impl AnthropicProvider { /// Apply cache control to the last message content block fn apply_cache_to_last_message(messages: &mut [NativeMessage]) { - if let Some(last_msg) = messages.last_mut() { - if let Some(last_content) = last_msg.content.last_mut() { - match last_content { - NativeContentOut::Text { cache_control, .. } - | NativeContentOut::ToolResult { cache_control, .. } => { - *cache_control = Some(CacheControl::ephemeral()); - } - NativeContentOut::ToolUse { .. } => {} + if let Some(last_msg) = messages.last_mut() + && let Some(last_content) = last_msg.content.last_mut() + { + match last_content { + NativeContentOut::Text { cache_control, .. } + | NativeContentOut::ToolResult { cache_control, .. } => { + *cache_control = Some(CacheControl::ephemeral()); } + NativeContentOut::ToolUse { .. } => {} } } } @@ -385,10 +388,10 @@ impl AnthropicProvider { for block in response.content { match block.kind.as_str() { "text" => { - if let Some(text) = block.text.map(|t| t.trim().to_string()) { - if !text.is_empty() { - text_parts.push(text); - } + if let Some(text) = block.text.map(|t| t.trim().to_string()) + && !text.is_empty() + { + text_parts.push(text); } } "tool_use" => { @@ -518,10 +521,10 @@ impl Provider for AnthropicProvider { .header("content-type", "application/json") .json(&native_request); - if let Some(tools) = &native_request.tools { - if !tools.is_empty() { - req = req.header("anthropic-beta", "prompt-caching-2024-07-31"); - } + if let Some(tools) = &native_request.tools + && !tools.is_empty() + { + req = req.header("anthropic-beta", "prompt-caching-2024-07-31"); } req = self.apply_auth(req, credential); @@ -588,10 +591,12 @@ mod tests { .chat_with_system(None, "hello", "claude-sonnet-4", 0.7) .await; assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("credentials not set")); + assert!( + result + .unwrap_err() + .to_string() + .contains("credentials not set") + ); } #[test] @@ -654,16 +659,20 @@ mod tests { let (_, native) = AnthropicProvider::convert_messages(&messages); assert_eq!(native.len(), 2); // First message should contain ToolUse block - assert!(native[0] - .content - .iter() - .any(|c| matches!(c, NativeContentOut::ToolUse { .. }))); + assert!( + native[0] + .content + .iter() + .any(|c| matches!(c, NativeContentOut::ToolUse { .. })) + ); // Second message (tool result) becomes a user message with ToolResult block assert_eq!(native[1].role, "user"); - assert!(native[1] - .content - .iter() - .any(|c| matches!(c, NativeContentOut::ToolResult { .. }))); + assert!( + native[1] + .content + .iter() + .any(|c| matches!(c, NativeContentOut::ToolResult { .. })) + ); } #[test] diff --git a/crewforge-rs/src/provider/compatible.rs b/crewforge-rs/src/provider/compatible.rs index dc4463e..0b49107 100644 --- a/crewforge-rs/src/provider/compatible.rs +++ b/crewforge-rs/src/provider/compatible.rs @@ -2,16 +2,15 @@ //! Most LLM APIs follow the same `/v1/chat/completions` format. //! This module provides a single implementation that works for all of them. +use crate::provider::traits::ToolSpec; use crate::provider::traits::{ ChatMessage, ChatRequest as ProviderChatRequest, ChatResponse as ProviderChatResponse, - Provider, TokenUsage, - ToolCall as ProviderToolCall, + Provider, TokenUsage, ToolCall as ProviderToolCall, }; -use crate::provider::traits::ToolSpec; use async_trait::async_trait; use reqwest::{ - header::{HeaderMap, HeaderValue, USER_AGENT}, Client, + header::{HeaderMap, HeaderValue, USER_AGENT}, }; use serde::{Deserialize, Serialize}; @@ -149,6 +148,7 @@ impl OpenAiCompatibleProvider { ) } + #[allow(clippy::too_many_arguments)] fn new_with_options( name: &str, base_url: &str, @@ -289,6 +289,7 @@ impl OpenAiCompatibleProvider { } } + #[allow(dead_code)] fn tool_specs_to_openai_format(tools: &[ToolSpec]) -> Vec { tools .iter() @@ -448,10 +449,10 @@ impl ToolCall { /// Extract function name with fallback logic for various provider formats fn function_name(&self) -> Option { // Standard OpenAI format: tool_calls[].function.name - if let Some(ref func) = self.function { - if let Some(ref name) = func.name { - return Some(name.clone()); - } + if let Some(ref func) = self.function + && let Some(ref name) = func.name + { + return Some(name.clone()); } // Fallback: direct name field self.name.clone() @@ -460,10 +461,10 @@ impl ToolCall { /// Extract arguments with fallback logic and type conversion fn function_arguments(&self) -> Option { // Standard OpenAI format: tool_calls[].function.arguments (string) - if let Some(ref func) = self.function { - if let Some(ref args) = func.arguments { - return Some(args.clone()); - } + if let Some(ref func) = self.function + && let Some(ref args) = func.arguments + { + return Some(args.clone()); } // Fallback: direct arguments field if let Some(ref args) = self.arguments { @@ -605,10 +606,10 @@ fn extract_responses_text(response: ResponsesResponse) -> Option { for item in &response.output { for content in &item.content { - if content.kind.as_deref() == Some("output_text") { - if let Some(text) = first_nonempty(content.text.as_deref()) { - return Some(text); - } + if content.kind.as_deref() == Some("output_text") + && let Some(text) = first_nonempty(content.text.as_deref()) + { + return Some(text); } } } @@ -705,9 +706,7 @@ impl OpenAiCompatibleProvider { .ok_or_else(|| anyhow::anyhow!("No response from {} Responses API", self.name)) } - fn convert_tool_specs( - tools: Option<&[ToolSpec]>, - ) -> Option> { + fn convert_tool_specs(tools: Option<&[ToolSpec]>) -> Option> { tools.map(|items| { items .iter() @@ -725,82 +724,75 @@ impl OpenAiCompatibleProvider { }) } + #[allow(dead_code)] fn to_message_content(_role: &str, content: &str) -> MessageContent { MessageContent::Text(content.to_string()) } - fn convert_messages_for_native( - messages: &[ChatMessage], - ) -> Vec { + fn convert_messages_for_native(messages: &[ChatMessage]) -> Vec { messages .iter() .map(|message| { - if message.role == "assistant" { - if let Ok(value) = serde_json::from_str::(&message.content) - { - if let Some(tool_calls_value) = value.get("tool_calls") { - if let Ok(parsed_calls) = - serde_json::from_value::>( - tool_calls_value.clone(), - ) - { - let tool_calls = parsed_calls - .into_iter() - .map(|tc| ToolCall { - id: Some(tc.id), - kind: Some("function".to_string()), - function: Some(Function { - name: Some(tc.name), - arguments: Some(tc.arguments), - }), - name: None, - arguments: None, - parameters: None, - }) - .collect::>(); - - let content = value - .get("content") - .and_then(serde_json::Value::as_str) - .map(|value| MessageContent::Text(value.to_string())); - - let reasoning_content = value - .get("reasoning_content") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - - return NativeMessage { - role: "assistant".to_string(), - content, - tool_call_id: None, - tool_calls: Some(tool_calls), - reasoning_content, - }; - } - } - } + if message.role == "assistant" + && let Ok(value) = serde_json::from_str::(&message.content) + && let Some(tool_calls_value) = value.get("tool_calls") + && let Ok(parsed_calls) = + serde_json::from_value::>(tool_calls_value.clone()) + { + let tool_calls = parsed_calls + .into_iter() + .map(|tc| ToolCall { + id: Some(tc.id), + kind: Some("function".to_string()), + function: Some(Function { + name: Some(tc.name), + arguments: Some(tc.arguments), + }), + name: None, + arguments: None, + parameters: None, + }) + .collect::>(); + + let content = value + .get("content") + .and_then(serde_json::Value::as_str) + .map(|value| MessageContent::Text(value.to_string())); + + let reasoning_content = value + .get("reasoning_content") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + + return NativeMessage { + role: "assistant".to_string(), + content, + tool_call_id: None, + tool_calls: Some(tool_calls), + reasoning_content, + }; } - if message.role == "tool" { - if let Ok(value) = serde_json::from_str::(&message.content) { - let tool_call_id = value - .get("tool_call_id") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - let content = value - .get("content") - .and_then(serde_json::Value::as_str) - .map(|value| MessageContent::Text(value.to_string())) - .or_else(|| Some(MessageContent::Text(message.content.clone()))); - - return NativeMessage { - role: "tool".to_string(), - content, - tool_call_id, - tool_calls: None, - reasoning_content: None, - }; - } + if message.role == "tool" + && let Ok(value) = serde_json::from_str::(&message.content) + { + let tool_call_id = value + .get("tool_call_id") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + let content = value + .get("content") + .and_then(serde_json::Value::as_str) + .map(|value| MessageContent::Text(value.to_string())) + .or_else(|| Some(MessageContent::Text(message.content.clone()))); + + return NativeMessage { + role: "tool".to_string(), + content, + tool_call_id, + tool_calls: None, + reasoning_content: None, + }; } NativeMessage { @@ -1063,10 +1055,7 @@ impl Provider for OpenAiCompatibleProvider { // If tool_calls are present, serialize the full message as JSON // so parse_tool_calls can handle the OpenAI-style format if c.message.tool_calls.is_some() - && c.message - .tool_calls - .as_ref() - .map_or(false, |t| !t.is_empty()) + && c.message.tool_calls.as_ref().is_some_and(|t| !t.is_empty()) { serde_json::to_string(&c.message) .unwrap_or_else(|_| c.message.effective_content()) @@ -1168,10 +1157,7 @@ impl Provider for OpenAiCompatibleProvider { // If tool_calls are present, serialize the full message as JSON // so parse_tool_calls can handle the OpenAI-style format if c.message.tool_calls.is_some() - && c.message - .tool_calls - .as_ref() - .map_or(false, |t| !t.is_empty()) + && c.message.tool_calls.as_ref().is_some_and(|t| !t.is_empty()) { serde_json::to_string(&c.message) .unwrap_or_else(|_| c.message.effective_content()) @@ -1313,9 +1299,7 @@ impl Provider for OpenAiCompatibleProvider { }; let native_request = NativeChatRequest { model: model.to_string(), - messages: Self::convert_messages_for_native( - &effective_messages, - ), + messages: Self::convert_messages_for_native(&effective_messages), temperature, stream: Some(false), tool_choice: tools.as_ref().map(|_| "auto".to_string()), @@ -1471,10 +1455,12 @@ mod tests { .chat_with_system(None, "hello", "llama-3.3-70b", 0.7) .await; assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("Venice API key not set")); + assert!( + result + .unwrap_err() + .to_string() + .contains("Venice API key not set") + ); } #[test] @@ -1654,9 +1640,10 @@ mod tests { .await .expect_err("system-only fallback payload should fail"); - assert!(err - .to_string() - .contains("requires at least one non-system message")); + assert!( + err.to_string() + .contains("requires at least one non-system message") + ); } #[test] @@ -1967,7 +1954,10 @@ mod tests { OpenAiCompatibleProvider::with_prompt_guided_tool_instructions(&input, Some(&tools)); assert!(!output.is_empty()); assert_eq!(output[0].role, "system"); - assert!(output[0].content.contains("Available Tools") || output[0].content.contains("Tool Use Protocol")); + assert!( + output[0].content.contains("Available Tools") + || output[0].content.contains("Tool Use Protocol") + ); assert!(output[0].content.contains("shell_exec")); } @@ -2129,10 +2119,12 @@ mod tests { let result = p.chat_with_tools(&messages, &tools, "model", 0.7).await; assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("TestProvider API key not set")); + assert!( + result + .unwrap_err() + .to_string() + .contains("TestProvider API key not set") + ); } #[test] diff --git a/crewforge-rs/src/provider/copilot.rs b/crewforge-rs/src/provider/copilot.rs index b6dea7e..08ed40d 100644 --- a/crewforge-rs/src/provider/copilot.rs +++ b/crewforge-rs/src/provider/copilot.rs @@ -252,58 +252,55 @@ impl CopilotProvider { messages .iter() .map(|message| { - if message.role == "assistant" { - if let Ok(value) = serde_json::from_str::(&message.content) { - if let Some(tool_calls_value) = value.get("tool_calls") { - if let Ok(parsed_calls) = - serde_json::from_value::>(tool_calls_value.clone()) - { - let tool_calls = parsed_calls - .into_iter() - .map(|tool_call| NativeToolCall { - id: Some(tool_call.id), - kind: Some("function".to_string()), - function: NativeFunctionCall { - name: tool_call.name, - arguments: tool_call.arguments, - }, - }) - .collect::>(); - - let content = value - .get("content") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - - return ApiMessage { - role: "assistant".to_string(), - content, - tool_call_id: None, - tool_calls: Some(tool_calls), - }; - } - } - } + if message.role == "assistant" + && let Ok(value) = serde_json::from_str::(&message.content) + && let Some(tool_calls_value) = value.get("tool_calls") + && let Ok(parsed_calls) = + serde_json::from_value::>(tool_calls_value.clone()) + { + let tool_calls = parsed_calls + .into_iter() + .map(|tool_call| NativeToolCall { + id: Some(tool_call.id), + kind: Some("function".to_string()), + function: NativeFunctionCall { + name: tool_call.name, + arguments: tool_call.arguments, + }, + }) + .collect::>(); + + let content = value + .get("content") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + + return ApiMessage { + role: "assistant".to_string(), + content, + tool_call_id: None, + tool_calls: Some(tool_calls), + }; } - if message.role == "tool" { - if let Ok(value) = serde_json::from_str::(&message.content) { - let tool_call_id = value - .get("tool_call_id") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - let content = value - .get("content") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - - return ApiMessage { - role: "tool".to_string(), - content, - tool_call_id, - tool_calls: None, - }; - } + if message.role == "tool" + && let Ok(value) = serde_json::from_str::(&message.content) + { + let tool_call_id = value + .get("tool_call_id") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + let content = value + .get("content") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + + return ApiMessage { + role: "tool".to_string(), + content, + tool_call_id, + tool_calls: None, + }; } ApiMessage { @@ -390,28 +387,28 @@ impl CopilotProvider { async fn get_api_key(&self) -> anyhow::Result<(String, String)> { let mut cached = self.refresh_lock.lock().await; - if let Some(cached_key) = cached.as_ref() { - if chrono::Utc::now().timestamp() + 120 < cached_key.expires_at { - return Ok((cached_key.token.clone(), cached_key.api_endpoint.clone())); - } + if let Some(cached_key) = cached.as_ref() + && chrono::Utc::now().timestamp() + 120 < cached_key.expires_at + { + return Ok((cached_key.token.clone(), cached_key.api_endpoint.clone())); } - if let Some(info) = self.load_api_key_from_disk().await { - if chrono::Utc::now().timestamp() + 120 < info.expires_at { - let endpoint = info - .endpoints - .as_ref() - .and_then(|e| e.api.clone()) - .unwrap_or_else(|| DEFAULT_API.to_string()); - let token = info.token; - - *cached = Some(CachedApiKey { - token: token.clone(), - api_endpoint: endpoint.clone(), - expires_at: info.expires_at, - }); - return Ok((token, endpoint)); - } + if let Some(info) = self.load_api_key_from_disk().await + && chrono::Utc::now().timestamp() + 120 < info.expires_at + { + let endpoint = info + .endpoints + .as_ref() + .and_then(|e| e.api.clone()) + .unwrap_or_else(|| DEFAULT_API.to_string()); + let token = info.token; + + *cached = Some(CachedApiKey { + token: token.clone(), + api_endpoint: endpoint.clone(), + expires_at: info.expires_at, + }); + return Ok((token, endpoint)); } let access_token = self.get_github_access_token().await?; @@ -702,12 +699,16 @@ mod tests { #[test] fn copilot_headers_include_required_fields() { let headers = CopilotProvider::COPILOT_HEADERS; - assert!(headers - .iter() - .any(|(header, _)| *header == "Editor-Version")); - assert!(headers - .iter() - .any(|(header, _)| *header == "Editor-Plugin-Version")); + assert!( + headers + .iter() + .any(|(header, _)| *header == "Editor-Version") + ); + assert!( + headers + .iter() + .any(|(header, _)| *header == "Editor-Plugin-Version") + ); assert!(headers.iter().any(|(header, _)| *header == "User-Agent")); } diff --git a/crewforge-rs/src/provider/gemini.rs b/crewforge-rs/src/provider/gemini.rs index 7c5cb5e..d6196e5 100644 --- a/crewforge-rs/src/provider/gemini.rs +++ b/crewforge-rs/src/provider/gemini.rs @@ -205,12 +205,7 @@ impl GeminiProvider { let url = self.build_generate_content_url(model); - let response = self - .http_client() - .post(&url) - .json(&request) - .send() - .await?; + let response = self.http_client().post(&url).json(&request).send().await?; if !response.status().is_success() { let status = response.status(); @@ -464,7 +459,12 @@ mod tests { .chat_with_system(None, "hello", "gemini-2.5-pro", 0.7) .await; assert!(result.is_err()); - assert!(result.unwrap_err().to_string().contains("API key not found")); + assert!( + result + .unwrap_err() + .to_string() + .contains("API key not found") + ); if let Some(v) = old_gemini { unsafe { std::env::set_var("GEMINI_API_KEY", v) }; } diff --git a/crewforge-rs/src/provider/glm.rs b/crewforge-rs/src/provider/glm.rs index f3ed2ef..066e49b 100644 --- a/crewforge-rs/src/provider/glm.rs +++ b/crewforge-rs/src/provider/glm.rs @@ -53,8 +53,16 @@ fn base64url_encode_bytes(data: &[u8]) -> String { let mut i = 0; while i < data.len() { let b0 = data[i] as u32; - let b1 = if i + 1 < data.len() { data[i + 1] as u32 } else { 0 }; - let b2 = if i + 2 < data.len() { data[i + 2] as u32 } else { 0 }; + let b1 = if i + 1 < data.len() { + data[i + 1] as u32 + } else { + 0 + }; + let b2 = if i + 2 < data.len() { + data[i + 2] as u32 + } else { + 0 + }; let triple = (b0 << 16) | (b1 << 8) | b2; result.push(CHARS[((triple >> 18) & 0x3F) as usize] as char); @@ -101,17 +109,14 @@ impl GlmProvider { ); } - let now_ms = SystemTime::now() - .duration_since(UNIX_EPOCH)? - .as_millis() as u64; + let now_ms = SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() as u64; // Check cache (valid for 3 minutes, token expires at 3.5 min) - if let Ok(cache) = self.token_cache.lock() { - if let Some((ref token, expiry)) = *cache { - if now_ms < expiry { - return Ok(token.clone()); - } - } + if let Ok(cache) = self.token_cache.lock() + && let Some((ref token, expiry)) = *cache + && now_ms < expiry + { + return Ok(token.clone()); } let exp_ms = now_ms + 210_000; // 3.5 minutes diff --git a/crewforge-rs/src/provider/mod.rs b/crewforge-rs/src/provider/mod.rs index 8c000eb..adb10a8 100644 --- a/crewforge-rs/src/provider/mod.rs +++ b/crewforge-rs/src/provider/mod.rs @@ -12,13 +12,13 @@ pub mod router; pub mod traits; pub use traits::{ - build_tool_instructions_text, ChatMessage, ChatRequest, ChatResponse, ConversationMessage, - Provider, ProviderCapabilities, ToolCall, ToolResultMessage, ToolSpec, TokenUsage, + ChatMessage, ChatRequest, ChatResponse, ConversationMessage, Provider, ProviderCapabilities, + TokenUsage, ToolCall, ToolResultMessage, ToolSpec, build_tool_instructions_text, }; +pub use compatible::{api_error, sanitize_api_error}; pub use reliable::ReliableProvider; pub use router::{Route, RouterProvider}; -pub use compatible::{api_error, sanitize_api_error}; use compatible::{AuthStyle, OpenAiCompatibleProvider}; @@ -50,7 +50,10 @@ pub fn create_provider( resolved_key.as_deref(), )), "gemini" | "google" => Box::new(gemini::GeminiProvider::new(resolved_key.as_deref())), - "ollama" => Box::new(ollama::OllamaProvider::new(base_url, resolved_key.as_deref())), + "ollama" => Box::new(ollama::OllamaProvider::new( + base_url, + resolved_key.as_deref(), + )), "openrouter" => Box::new(openrouter::OpenRouterProvider::new(resolved_key.as_deref())), "glm" | "zhipuai" | "zhipu" => Box::new(glm::GlmProvider::new(resolved_key.as_deref())), "moonshot" | "kimi" => Box::new(OpenAiCompatibleProvider::new( @@ -128,10 +131,10 @@ pub fn create_provider( } fn resolve_api_key(provider_name: &str, explicit: Option<&str>) -> Option { - if let Some(k) = explicit { - if !k.is_empty() { - return Some(k.to_string()); - } + if let Some(k) = explicit + && !k.is_empty() + { + return Some(k.to_string()); } let env_var = default_api_key_env(provider_name)?; std::env::var(env_var).ok().filter(|k| !k.is_empty()) diff --git a/crewforge-rs/src/provider/ollama.rs b/crewforge-rs/src/provider/ollama.rs index a66eba3..d7786f2 100644 --- a/crewforge-rs/src/provider/ollama.rs +++ b/crewforge-rs/src/provider/ollama.rs @@ -93,11 +93,7 @@ struct OllamaFunction { // ─── Implementation ─────────────────────────────────────────────────────────── fn sanitize_api_error(raw: &str) -> String { - let truncated = if raw.len() > 500 { - &raw[..500] - } else { - raw - }; + let truncated = if raw.len() > 500 { &raw[..500] } else { raw }; truncated.replace('\n', " ").trim().to_string() } @@ -232,71 +228,65 @@ impl OllamaProvider { messages .iter() .map(|message| { - if message.role == "assistant" { - if let Ok(value) = serde_json::from_str::(&message.content) { - if let Some(tool_calls_value) = value.get("tool_calls") { - if let Ok(parsed_calls) = - serde_json::from_value::>(tool_calls_value.clone()) - { - let outgoing_calls: Vec = parsed_calls - .into_iter() - .map(|call| { - tool_name_by_id.insert(call.id.clone(), call.name.clone()); - OutgoingToolCall { - kind: "function".to_string(), - function: OutgoingFunction { - name: call.name, - arguments: Self::parse_tool_arguments( - &call.arguments, - ), - }, - } - }) - .collect(); - let content = value - .get("content") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - return Message { - role: "assistant".to_string(), - content, - tool_calls: Some(outgoing_calls), - tool_name: None, - }; + if message.role == "assistant" + && let Ok(value) = serde_json::from_str::(&message.content) + && let Some(tool_calls_value) = value.get("tool_calls") + && let Ok(parsed_calls) = + serde_json::from_value::>(tool_calls_value.clone()) + { + let outgoing_calls: Vec = parsed_calls + .into_iter() + .map(|call| { + tool_name_by_id.insert(call.id.clone(), call.name.clone()); + OutgoingToolCall { + kind: "function".to_string(), + function: OutgoingFunction { + name: call.name, + arguments: Self::parse_tool_arguments(&call.arguments), + }, } - } - } + }) + .collect(); + let content = value + .get("content") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + return Message { + role: "assistant".to_string(), + content, + tool_calls: Some(outgoing_calls), + tool_name: None, + }; } - if message.role == "tool" { - if let Ok(value) = serde_json::from_str::(&message.content) { - let tool_name = value - .get("tool_name") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string) - .or_else(|| { - value - .get("tool_call_id") - .and_then(serde_json::Value::as_str) - .and_then(|id| tool_name_by_id.get(id)) - .cloned() - }); - let content = value - .get("content") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string) - .or_else(|| { - (!message.content.trim().is_empty()) - .then_some(message.content.clone()) - }); - - return Message { - role: "tool".to_string(), - content, - tool_calls: None, - tool_name, - }; - } + if message.role == "tool" + && let Ok(value) = serde_json::from_str::(&message.content) + { + let tool_name = value + .get("tool_name") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string) + .or_else(|| { + value + .get("tool_call_id") + .and_then(serde_json::Value::as_str) + .and_then(|id| tool_name_by_id.get(id)) + .cloned() + }); + let content = value + .get("content") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string) + .or_else(|| { + (!message.content.trim().is_empty()).then_some(message.content.clone()) + }); + + return Message { + role: "tool".to_string(), + content, + tool_calls: None, + tool_name, + }; } Message { @@ -334,10 +324,8 @@ impl OllamaProvider { let mut request_builder = self.http_client().post(&url).json(&request); - if should_auth { - if let Some(key) = self.api_key.as_ref() { - request_builder = request_builder.bearer_auth(key); - } + if should_auth && let Some(key) = self.api_key.as_ref() { + request_builder = request_builder.bearer_auth(key); } let response = request_builder.send().await?; @@ -412,24 +400,23 @@ impl OllamaProvider { let args = &tc.function.arguments; // Pattern 1: Nested tool_call wrapper - if name == "tool_call" + if (name == "tool_call" || name == "tool.call" || name.starts_with("tool_call>") - || name.starts_with("tool_call<") + || name.starts_with("tool_call<")) + && let Some(nested_name) = args.get("name").and_then(|v| v.as_str()) { - if let Some(nested_name) = args.get("name").and_then(|v| v.as_str()) { - let nested_args = args - .get("arguments") - .cloned() - .unwrap_or(serde_json::json!({})); - tracing::debug!( - "Unwrapped nested tool call: {} -> {} with args {:?}", - name, - nested_name, - nested_args - ); - return (nested_name.to_string(), nested_args); - } + let nested_args = args + .get("arguments") + .cloned() + .unwrap_or(serde_json::json!({})); + tracing::debug!( + "Unwrapped nested tool call: {} -> {} with args {:?}", + name, + nested_name, + nested_args + ); + return (nested_name.to_string(), nested_args); } // Pattern 2: Prefixed tool name (tool.shell, tool.file_read, etc.) @@ -634,25 +621,25 @@ impl Provider for OllamaProvider { temperature: f64, ) -> anyhow::Result { // Convert ToolSpec to OpenAI-compatible JSON and delegate to chat_with_tools. - if let Some(specs) = request.tools { - if !specs.is_empty() { - let tools: Vec = specs - .iter() - .map(|s| { - serde_json::json!({ - "type": "function", - "function": { - "name": s.name, - "description": s.description, - "parameters": s.parameters - } - }) + if let Some(specs) = request.tools + && !specs.is_empty() + { + let tools: Vec = specs + .iter() + .map(|s| { + serde_json::json!({ + "type": "function", + "function": { + "name": s.name, + "description": s.description, + "parameters": s.parameters + } }) - .collect(); - return self - .chat_with_tools(request.messages, &tools, model, temperature) - .await; - } + }) + .collect(); + return self + .chat_with_tools(request.messages, &tools, model, temperature) + .await; } // No tools — fall back to plain text chat. @@ -718,9 +705,11 @@ mod tests { let error = p .resolve_request_details("qwen3:cloud") .expect_err("cloud suffix should fail on local endpoint"); - assert!(error - .to_string() - .contains("requested cloud routing, but Ollama endpoint is local")); + assert!( + error + .to_string() + .contains("requested cloud routing, but Ollama endpoint is local") + ); } #[test] @@ -729,9 +718,11 @@ mod tests { let error = p .resolve_request_details("qwen3:cloud") .expect_err("cloud suffix should require API key"); - assert!(error - .to_string() - .contains("requested cloud routing, but no API key is configured")); + assert!( + error + .to_string() + .contains("requested cloud routing, but no API key is configured") + ); } #[test] diff --git a/crewforge-rs/src/provider/openai.rs b/crewforge-rs/src/provider/openai.rs index 2d67122..7e4e68a 100644 --- a/crewforge-rs/src/provider/openai.rs +++ b/crewforge-rs/src/provider/openai.rs @@ -206,63 +206,58 @@ impl OpenAiProvider { messages .iter() .map(|m| { - if m.role == "assistant" { - if let Ok(value) = serde_json::from_str::(&m.content) { - if let Some(tool_calls_value) = value.get("tool_calls") { - if let Ok(parsed_calls) = - serde_json::from_value::>( - tool_calls_value.clone(), - ) - { - let tool_calls = parsed_calls - .into_iter() - .map(|tc| NativeToolCall { - id: Some(tc.id), - kind: Some("function".to_string()), - function: NativeFunctionCall { - name: tc.name, - arguments: tc.arguments, - }, - }) - .collect::>(); - let content = value - .get("content") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - let reasoning_content = value - .get("reasoning_content") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - return NativeMessage { - role: "assistant".to_string(), - content, - tool_call_id: None, - tool_calls: Some(tool_calls), - reasoning_content, - }; - } - } - } + if m.role == "assistant" + && let Ok(value) = serde_json::from_str::(&m.content) + && let Some(tool_calls_value) = value.get("tool_calls") + && let Ok(parsed_calls) = + serde_json::from_value::>(tool_calls_value.clone()) + { + let tool_calls = parsed_calls + .into_iter() + .map(|tc| NativeToolCall { + id: Some(tc.id), + kind: Some("function".to_string()), + function: NativeFunctionCall { + name: tc.name, + arguments: tc.arguments, + }, + }) + .collect::>(); + let content = value + .get("content") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + let reasoning_content = value + .get("reasoning_content") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + return NativeMessage { + role: "assistant".to_string(), + content, + tool_call_id: None, + tool_calls: Some(tool_calls), + reasoning_content, + }; } - if m.role == "tool" { - if let Ok(value) = serde_json::from_str::(&m.content) { - let tool_call_id = value - .get("tool_call_id") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - let content = value - .get("content") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - return NativeMessage { - role: "tool".to_string(), - content, - tool_call_id, - tool_calls: None, - reasoning_content: None, - }; - } + if m.role == "tool" + && let Ok(value) = serde_json::from_str::(&m.content) + { + let tool_call_id = value + .get("tool_call_id") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + let content = value + .get("content") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + return NativeMessage { + role: "tool".to_string(), + content, + tool_call_id, + tool_calls: None, + reasoning_content: None, + }; } NativeMessage { @@ -694,10 +689,12 @@ mod tests { let result = p.chat_with_tools(&messages, &tools, "gpt-4o", 0.7).await; assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("Invalid OpenAI tool specification")); + assert!( + result + .unwrap_err() + .to_string() + .contains("Invalid OpenAI tool specification") + ); } #[test] diff --git a/crewforge-rs/src/provider/openai_codex.rs b/crewforge-rs/src/provider/openai_codex.rs index 71a40e0..a6edf78 100644 --- a/crewforge-rs/src/provider/openai_codex.rs +++ b/crewforge-rs/src/provider/openai_codex.rs @@ -1,7 +1,7 @@ -use crate::auth::openai_oauth::extract_account_id_from_jwt; use crate::auth::AuthService; -use crate::provider::traits::{ChatMessage, Provider, ProviderCapabilities}; +use crate::auth::openai_oauth::extract_account_id_from_jwt; use crate::provider::ProviderRuntimeOptions; +use crate::provider::traits::{ChatMessage, Provider, ProviderCapabilities}; use async_trait::async_trait; use reqwest::Client; use serde::{Deserialize, Serialize}; @@ -198,6 +198,7 @@ fn first_nonempty(text: Option<&str>) -> Option { }) } +#[allow(dead_code)] fn resolve_instructions(system_prompt: Option<&str>) -> String { first_nonempty(system_prompt).unwrap_or_else(|| DEFAULT_CODEX_INSTRUCTIONS.to_string()) } @@ -315,10 +316,10 @@ fn extract_responses_text(response: &ResponsesResponse) -> Option { for item in &response.output { for content in &item.content { - if content.kind.as_deref() == Some("output_text") { - if let Some(text) = first_nonempty(content.text.as_deref()) { - return Some(text); - } + if content.kind.as_deref() == Some("output_text") + && let Some(text) = first_nonempty(content.text.as_deref()) + { + return Some(text); } } } diff --git a/crewforge-rs/src/provider/openrouter.rs b/crewforge-rs/src/provider/openrouter.rs index 3dae0a5..5887d6b 100644 --- a/crewforge-rs/src/provider/openrouter.rs +++ b/crewforge-rs/src/provider/openrouter.rs @@ -164,64 +164,59 @@ impl OpenRouterProvider { messages .iter() .map(|m| { - if m.role == "assistant" { - if let Ok(value) = serde_json::from_str::(&m.content) { - if let Some(tool_calls_value) = value.get("tool_calls") { - if let Ok(parsed_calls) = - serde_json::from_value::>( - tool_calls_value.clone(), - ) - { - let tool_calls = parsed_calls - .into_iter() - .map(|tc| NativeToolCall { - id: Some(tc.id), - kind: Some("function".to_string()), - function: NativeFunctionCall { - name: tc.name, - arguments: tc.arguments, - }, - }) - .collect::>(); - let content = value - .get("content") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - let reasoning_content = value - .get("reasoning_content") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - return NativeMessage { - role: "assistant".to_string(), - content, - tool_call_id: None, - tool_calls: Some(tool_calls), - reasoning_content, - }; - } - } - } + if m.role == "assistant" + && let Ok(value) = serde_json::from_str::(&m.content) + && let Some(tool_calls_value) = value.get("tool_calls") + && let Ok(parsed_calls) = + serde_json::from_value::>(tool_calls_value.clone()) + { + let tool_calls = parsed_calls + .into_iter() + .map(|tc| NativeToolCall { + id: Some(tc.id), + kind: Some("function".to_string()), + function: NativeFunctionCall { + name: tc.name, + arguments: tc.arguments, + }, + }) + .collect::>(); + let content = value + .get("content") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + let reasoning_content = value + .get("reasoning_content") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + return NativeMessage { + role: "assistant".to_string(), + content, + tool_call_id: None, + tool_calls: Some(tool_calls), + reasoning_content, + }; } - if m.role == "tool" { - if let Ok(value) = serde_json::from_str::(&m.content) { - let tool_call_id = value - .get("tool_call_id") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - let content = value - .get("content") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string) - .or_else(|| Some(m.content.clone())); - return NativeMessage { - role: "tool".to_string(), - content, - tool_call_id, - tool_calls: None, - reasoning_content: None, - }; - } + if m.role == "tool" + && let Ok(value) = serde_json::from_str::(&m.content) + { + let tool_call_id = value + .get("tool_call_id") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string); + let content = value + .get("content") + .and_then(serde_json::Value::as_str) + .map(ToString::to_string) + .or_else(|| Some(m.content.clone())); + return NativeMessage { + role: "tool".to_string(), + content, + tool_call_id, + tool_calls: None, + reasoning_content: None, + }; } NativeMessage { @@ -292,10 +287,9 @@ impl Provider for OpenRouterProvider { model: &str, temperature: f64, ) -> anyhow::Result { - let credential = self - .credential - .as_ref() - .ok_or_else(|| anyhow::anyhow!("OpenRouter API key not set. Set OPENROUTER_API_KEY."))?; + let credential = self.credential.as_ref().ok_or_else(|| { + anyhow::anyhow!("OpenRouter API key not set. Set OPENROUTER_API_KEY.") + })?; let mut messages = Vec::new(); @@ -347,10 +341,9 @@ impl Provider for OpenRouterProvider { model: &str, temperature: f64, ) -> anyhow::Result { - let credential = self - .credential - .as_ref() - .ok_or_else(|| anyhow::anyhow!("OpenRouter API key not set. Set OPENROUTER_API_KEY."))?; + let credential = self.credential.as_ref().ok_or_else(|| { + anyhow::anyhow!("OpenRouter API key not set. Set OPENROUTER_API_KEY.") + })?; let api_messages: Vec = messages .iter() @@ -478,11 +471,7 @@ impl Provider for OpenRouterProvider { }) }) .collect(); - if specs.is_empty() { - None - } else { - Some(specs) - } + if specs.is_empty() { None } else { Some(specs) } }; let native_messages = Self::convert_messages(messages); diff --git a/crewforge-rs/src/provider/reliable.rs b/crewforge-rs/src/provider/reliable.rs index 2bb183d..8a2baac 100644 --- a/crewforge-rs/src/provider/reliable.rs +++ b/crewforge-rs/src/provider/reliable.rs @@ -9,8 +9,8 @@ //! Loop invariant: `failures` accumulates every failed attempt so the final //! error message gives operators a complete diagnostic trail. -use crate::provider::traits::{ChatMessage, ChatRequest, ChatResponse}; use crate::provider::Provider; +use crate::provider::traits::{ChatMessage, ChatRequest, ChatResponse}; use async_trait::async_trait; use std::collections::HashMap; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -30,20 +30,20 @@ fn is_non_retryable(err: &anyhow::Error) -> bool { // 4xx errors are generally non-retryable (bad request, auth failure, etc.), // except 429 (rate-limit — transient) and 408 (timeout — worth retrying). - if let Some(reqwest_err) = err.downcast_ref::() { - if let Some(status) = reqwest_err.status() { - let code = status.as_u16(); - return status.is_client_error() && code != 429 && code != 408; - } + if let Some(reqwest_err) = err.downcast_ref::() + && let Some(status) = reqwest_err.status() + { + let code = status.as_u16(); + return status.is_client_error() && code != 429 && code != 408; } // Fallback: parse status codes from stringified errors (some providers // embed codes in error messages rather than returning typed HTTP errors). let msg = err.to_string(); for word in msg.split(|c: char| !c.is_ascii_digit()) { - if let Ok(code) = word.parse::() { - if (400..500).contains(&code) { - return code != 429 && code != 408; - } + if let Ok(code) = word.parse::() + && (400..500).contains(&code) + { + return code != 429 && code != 408; } } @@ -97,10 +97,10 @@ fn is_context_window_exceeded(err: &anyhow::Error) -> bool { /// Check if an error is a rate-limit (429) error. fn is_rate_limited(err: &anyhow::Error) -> bool { - if let Some(reqwest_err) = err.downcast_ref::() { - if let Some(status) = reqwest_err.status() { - return status.as_u16() == 429; - } + if let Some(reqwest_err) = err.downcast_ref::() + && let Some(status) = reqwest_err.status() + { + return status.as_u16() == 429; } let msg = err.to_string(); msg.contains("429") @@ -143,10 +143,10 @@ fn is_non_retryable_rate_limit(err: &anyhow::Error) -> bool { // Known provider business codes observed for 429 where retry is futile. for token in lower.split(|c: char| !c.is_ascii_digit()) { - if let Ok(code) = token.parse::() { - if matches!(code, 1113 | 1311) { - return true; - } + if let Ok(code) = token.parse::() + && matches!(code, 1113 | 1311) + { + return true; } } @@ -173,12 +173,13 @@ fn parse_retry_after_ms(err: &anyhow::Error) -> Option { .chars() .take_while(|c| c.is_ascii_digit() || *c == '.') .collect(); - if let Ok(secs) = num_str.parse::() { - if secs.is_finite() && secs >= 0.0 { - let millis = Duration::from_secs_f64(secs).as_millis(); - if let Ok(value) = u64::try_from(millis) { - return Some(value); - } + if let Ok(secs) = num_str.parse::() + && secs.is_finite() + && secs >= 0.0 + { + let millis = Duration::from_secs_f64(secs).as_millis(); + if let Ok(value) = u64::try_from(millis) { + return Some(value); } } } @@ -410,17 +411,18 @@ impl Provider for ReliableProvider { // Rate-limit with rotatable keys: cycle to the next API key // so the retry hits a different quota bucket. - if rate_limited && !non_retryable_rate_limit { - if let Some(new_key) = self.rotate_key() { - tracing::warn!( - provider = provider_name, - error = %error_detail, - "Rate limited; key rotation selected key ending ...{} \ - but cannot apply (Provider trait has no set_api_key). \ - Retrying with original key.", - &new_key[new_key.len().saturating_sub(4)..] - ); - } + if rate_limited + && !non_retryable_rate_limit + && let Some(new_key) = self.rotate_key() + { + tracing::warn!( + provider = provider_name, + error = %error_detail, + "Rate limited; key rotation selected key ending ...{} \ + but cannot apply (Provider trait has no set_api_key). \ + Retrying with original key.", + &new_key[new_key.len().saturating_sub(4)..] + ); } if non_retryable { @@ -528,17 +530,18 @@ impl Provider for ReliableProvider { &error_detail, ); - if rate_limited && !non_retryable_rate_limit { - if let Some(new_key) = self.rotate_key() { - tracing::warn!( - provider = provider_name, - error = %error_detail, - "Rate limited; key rotation selected key ending ...{} \ - but cannot apply (Provider trait has no set_api_key). \ - Retrying with original key.", - &new_key[new_key.len().saturating_sub(4)..] - ); - } + if rate_limited + && !non_retryable_rate_limit + && let Some(new_key) = self.rotate_key() + { + tracing::warn!( + provider = provider_name, + error = %error_detail, + "Rate limited; key rotation selected key ending ...{} \ + but cannot apply (Provider trait has no set_api_key). \ + Retrying with original key.", + &new_key[new_key.len().saturating_sub(4)..] + ); } if non_retryable { @@ -652,17 +655,18 @@ impl Provider for ReliableProvider { &error_detail, ); - if rate_limited && !non_retryable_rate_limit { - if let Some(new_key) = self.rotate_key() { - tracing::warn!( - provider = provider_name, - error = %error_detail, - "Rate limited; key rotation selected key ending ...{} \ - but cannot apply (Provider trait has no set_api_key). \ - Retrying with original key.", - &new_key[new_key.len().saturating_sub(4)..] - ); - } + if rate_limited + && !non_retryable_rate_limit + && let Some(new_key) = self.rotate_key() + { + tracing::warn!( + provider = provider_name, + error = %error_detail, + "Rate limited; key rotation selected key ending ...{} \ + but cannot apply (Provider trait has no set_api_key). \ + Retrying with original key.", + &new_key[new_key.len().saturating_sub(4)..] + ); } if non_retryable { @@ -763,17 +767,18 @@ impl Provider for ReliableProvider { &error_detail, ); - if rate_limited && !non_retryable_rate_limit { - if let Some(new_key) = self.rotate_key() { - tracing::warn!( - provider = provider_name, - error = %error_detail, - "Rate limited; key rotation selected key ending ...{} \ - but cannot apply (Provider trait has no set_api_key). \ - Retrying with original key.", - &new_key[new_key.len().saturating_sub(4)..] - ); - } + if rate_limited + && !non_retryable_rate_limit + && let Some(new_key) = self.rotate_key() + { + tracing::warn!( + provider = provider_name, + error = %error_detail, + "Rate limited; key rotation selected key ending ...{} \ + but cannot apply (Provider trait has no set_api_key). \ + Retrying with original key.", + &new_key[new_key.len().saturating_sub(4)..] + ); } if non_retryable { diff --git a/crewforge-rs/src/provider/router.rs b/crewforge-rs/src/provider/router.rs index 167f544..f8e8888 100644 --- a/crewforge-rs/src/provider/router.rs +++ b/crewforge-rs/src/provider/router.rs @@ -1,8 +1,8 @@ //! Multi-model router that dispatches requests to different provider+model //! combinations based on a task hint encoded in the model parameter. -use crate::provider::traits::{ChatMessage, ChatRequest, ChatResponse}; use crate::provider::Provider; +use crate::provider::traits::{ChatMessage, ChatRequest, ChatResponse}; use async_trait::async_trait; use std::collections::HashMap; @@ -25,6 +25,7 @@ pub struct RouterProvider { routes: HashMap, // hint → (provider_index, model) providers: Vec<(String, Box)>, default_index: usize, + #[allow(dead_code)] default_model: String, } diff --git a/crewforge-rs/src/provider/traits.rs b/crewforge-rs/src/provider/traits.rs index 7391019..3d668c0 100644 --- a/crewforge-rs/src/provider/traits.rs +++ b/crewforge-rs/src/provider/traits.rs @@ -235,32 +235,32 @@ pub trait Provider: Send + Sync { model: &str, temperature: f64, ) -> anyhow::Result { - if let Some(tools) = request.tools { - if !tools.is_empty() && !self.supports_native_tools() { - let tool_instructions = build_tool_instructions_text(tools); - let mut modified_messages = request.messages.to_vec(); - - if let Some(system_message) = - modified_messages.iter_mut().find(|m| m.role == "system") - { - if !system_message.content.is_empty() { - system_message.content.push_str("\n\n"); - } - system_message.content.push_str(&tool_instructions); - } else { - modified_messages.insert(0, ChatMessage::system(tool_instructions)); + if let Some(tools) = request.tools + && !tools.is_empty() + && !self.supports_native_tools() + { + let tool_instructions = build_tool_instructions_text(tools); + let mut modified_messages = request.messages.to_vec(); + + if let Some(system_message) = modified_messages.iter_mut().find(|m| m.role == "system") + { + if !system_message.content.is_empty() { + system_message.content.push_str("\n\n"); } - - let text = self - .chat_with_history(&modified_messages, model, temperature) - .await?; - return Ok(ChatResponse { - text: Some(text), - tool_calls: Vec::new(), - usage: None, - reasoning_content: None, - }); + system_message.content.push_str(&tool_instructions); + } else { + modified_messages.insert(0, ChatMessage::system(tool_instructions)); } + + let text = self + .chat_with_history(&modified_messages, model, temperature) + .await?; + return Ok(ChatResponse { + text: Some(text), + tool_calls: Vec::new(), + usage: None, + reasoning_content: None, + }); } let text = self diff --git a/crewforge-rs/src/security/mod.rs b/crewforge-rs/src/security/mod.rs index 55467d7..b14a582 100644 --- a/crewforge-rs/src/security/mod.rs +++ b/crewforge-rs/src/security/mod.rs @@ -1,2 +1,4 @@ +pub mod policy; pub mod secrets; +pub use policy::*; pub use secrets::SecretStore; diff --git a/crewforge-rs/src/security/policy.rs b/crewforge-rs/src/security/policy.rs new file mode 100644 index 0000000..d012d31 --- /dev/null +++ b/crewforge-rs/src/security/policy.rs @@ -0,0 +1,1400 @@ +use std::path::{Path, PathBuf}; +use std::sync::Mutex; +use std::time::Instant; + +/// How much autonomy the agent has. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum AutonomyLevel { + /// Read-only: can observe but not act + ReadOnly, + /// Supervised: acts but requires approval for risky operations + #[default] + Supervised, + /// Full: autonomous execution within policy bounds + Full, +} + +/// Risk score for shell command execution. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CommandRiskLevel { + Low, + Medium, + High, +} + +/// Classifies whether a tool operation is read-only or side-effecting. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ToolOperation { + Read, + Act, +} + +/// Sliding-window action tracker for rate limiting. +#[derive(Debug)] +pub struct ActionTracker { + actions: Mutex>, +} + +impl Default for ActionTracker { + fn default() -> Self { + Self { + actions: Mutex::new(Vec::new()), + } + } +} + +impl ActionTracker { + pub fn new() -> Self { + Self::default() + } + + /// Record an action and return the current count within the window. + pub fn record(&self) -> usize { + let mut actions = self.actions.lock().unwrap(); + let cutoff = Instant::now() + .checked_sub(std::time::Duration::from_secs(3600)) + .unwrap_or_else(Instant::now); + actions.retain(|t| *t > cutoff); + actions.push(Instant::now()); + actions.len() + } + + /// Count of actions in the current window without recording. + pub fn count(&self) -> usize { + let mut actions = self.actions.lock().unwrap(); + let cutoff = Instant::now() + .checked_sub(std::time::Duration::from_secs(3600)) + .unwrap_or_else(Instant::now); + actions.retain(|t| *t > cutoff); + actions.len() + } +} + +impl Clone for ActionTracker { + fn clone(&self) -> Self { + let actions = self.actions.lock().unwrap(); + Self { + actions: Mutex::new(actions.clone()), + } + } +} + +/// Security policy enforced on all tool executions. +#[derive(Debug, Clone)] +pub struct SecurityPolicy { + pub autonomy: AutonomyLevel, + pub workspace_dir: PathBuf, + pub workspace_only: bool, + pub allowed_commands: Vec, + pub forbidden_paths: Vec, + pub allowed_roots: Vec, + pub max_actions_per_hour: u32, + pub require_approval_for_medium_risk: bool, + pub block_high_risk_commands: bool, + pub shell_env_passthrough: Vec, + pub tracker: ActionTracker, +} + +impl Default for SecurityPolicy { + fn default() -> Self { + Self { + autonomy: AutonomyLevel::Supervised, + workspace_dir: PathBuf::from("."), + workspace_only: true, + allowed_commands: vec![ + "git".into(), + "npm".into(), + "cargo".into(), + "ls".into(), + "cat".into(), + "grep".into(), + "find".into(), + "echo".into(), + "pwd".into(), + "wc".into(), + "head".into(), + "tail".into(), + "date".into(), + ], + forbidden_paths: vec![ + "/etc".into(), + "/root".into(), + "/home".into(), + "/usr".into(), + "/bin".into(), + "/sbin".into(), + "/lib".into(), + "/opt".into(), + "/boot".into(), + "/dev".into(), + "/proc".into(), + "/sys".into(), + "/var".into(), + "/tmp".into(), + "~/.ssh".into(), + "~/.gnupg".into(), + "~/.aws".into(), + "~/.config".into(), + ], + allowed_roots: Vec::new(), + max_actions_per_hour: 60, + require_approval_for_medium_risk: true, + block_high_risk_commands: true, + shell_env_passthrough: vec![], + tracker: ActionTracker::new(), + } + } +} + +// ── Shell Command Parsing Utilities ───────────────────────────────────────── + +fn home_dir() -> Option { + std::env::var_os("HOME").map(PathBuf::from) +} + +fn expand_user_path(path: &str) -> PathBuf { + let home = home_dir(); + if let (true, Some(h)) = (path == "~", &home) { + return h.clone(); + } + if let (Some(stripped), Some(h)) = (path.strip_prefix("~/"), &home) { + return h.join(stripped); + } + PathBuf::from(path) +} + +/// Skip leading environment variable assignments (e.g. `FOO=bar cmd args`). +fn skip_env_assignments(s: &str) -> &str { + let mut rest = s; + loop { + let Some(word) = rest.split_whitespace().next() else { + return rest; + }; + if word.contains('=') + && word + .chars() + .next() + .is_some_and(|c| c.is_ascii_alphabetic() || c == '_') + { + rest = rest[word.len()..].trim_start(); + } else { + return rest; + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum QuoteState { + None, + Single, + Double, +} + +/// Split a shell command into sub-commands by unquoted separators. +fn split_unquoted_segments(command: &str) -> Vec { + let mut segments = Vec::new(); + let mut current = String::new(); + let mut quote = QuoteState::None; + let mut escaped = false; + let mut chars = command.chars().peekable(); + + let push_segment = |segments: &mut Vec, current: &mut String| { + let trimmed = current.trim(); + if !trimmed.is_empty() { + segments.push(trimmed.to_string()); + } + current.clear(); + }; + + while let Some(ch) = chars.next() { + match quote { + QuoteState::Single => { + if ch == '\'' { + quote = QuoteState::None; + } + current.push(ch); + } + QuoteState::Double => { + if escaped { + escaped = false; + current.push(ch); + continue; + } + if ch == '\\' { + escaped = true; + current.push(ch); + continue; + } + if ch == '"' { + quote = QuoteState::None; + } + current.push(ch); + } + QuoteState::None => { + if escaped { + escaped = false; + current.push(ch); + continue; + } + if ch == '\\' { + escaped = true; + current.push(ch); + continue; + } + + match ch { + '\'' => { + quote = QuoteState::Single; + current.push(ch); + } + '"' => { + quote = QuoteState::Double; + current.push(ch); + } + ';' | '\n' => push_segment(&mut segments, &mut current), + '|' => { + if chars.next_if_eq(&'|').is_some() { + // `||` + } + push_segment(&mut segments, &mut current); + } + '&' => { + if chars.next_if_eq(&'&').is_some() { + push_segment(&mut segments, &mut current); + } else { + current.push(ch); + } + } + _ => current.push(ch), + } + } + } + } + + let trimmed = current.trim(); + if !trimmed.is_empty() { + segments.push(trimmed.to_string()); + } + + segments +} + +/// Detect a single unquoted `&` operator (background/chain). `&&` is allowed. +fn contains_unquoted_single_ampersand(command: &str) -> bool { + let mut quote = QuoteState::None; + let mut escaped = false; + let mut chars = command.chars().peekable(); + + while let Some(ch) = chars.next() { + match quote { + QuoteState::Single => { + if ch == '\'' { + quote = QuoteState::None; + } + } + QuoteState::Double => { + if escaped { + escaped = false; + continue; + } + if ch == '\\' { + escaped = true; + continue; + } + if ch == '"' { + quote = QuoteState::None; + } + } + QuoteState::None => { + if escaped { + escaped = false; + continue; + } + if ch == '\\' { + escaped = true; + continue; + } + match ch { + '\'' => quote = QuoteState::Single, + '"' => quote = QuoteState::Double, + '&' => { + if chars.next_if_eq(&'&').is_none() { + return true; + } + } + _ => {} + } + } + } + } + + false +} + +/// Detect an unquoted character in a shell command. +fn contains_unquoted_char(command: &str, target: char) -> bool { + let mut quote = QuoteState::None; + let mut escaped = false; + + for ch in command.chars() { + match quote { + QuoteState::Single => { + if ch == '\'' { + quote = QuoteState::None; + } + } + QuoteState::Double => { + if escaped { + escaped = false; + continue; + } + if ch == '\\' { + escaped = true; + continue; + } + if ch == '"' { + quote = QuoteState::None; + } + } + QuoteState::None => { + if escaped { + escaped = false; + continue; + } + if ch == '\\' { + escaped = true; + continue; + } + match ch { + '\'' => quote = QuoteState::Single, + '"' => quote = QuoteState::Double, + _ if ch == target => return true, + _ => {} + } + } + } + } + + false +} + +pub(crate) fn is_valid_env_var_name(name: &str) -> bool { + !name.is_empty() + && name + .chars() + .next() + .is_some_and(|c| c.is_ascii_alphabetic() || c == '_') + && name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') +} + +/// Detect unquoted shell variable expansions that are not explicitly allowlisted. +fn contains_disallowed_unquoted_shell_variable_expansion( + command: &str, + allowed_vars: &[String], +) -> bool { + let mut quote = QuoteState::None; + let mut escaped = false; + let chars: Vec = command.chars().collect(); + let mut i = 0usize; + + while i < chars.len() { + let ch = chars[i]; + + match quote { + QuoteState::Single => { + if ch == '\'' { + quote = QuoteState::None; + } + i += 1; + continue; + } + QuoteState::Double => { + if escaped { + escaped = false; + i += 1; + continue; + } + if ch == '\\' { + escaped = true; + i += 1; + continue; + } + if ch == '"' { + quote = QuoteState::None; + i += 1; + continue; + } + } + QuoteState::None => { + if escaped { + escaped = false; + i += 1; + continue; + } + if ch == '\\' { + escaped = true; + i += 1; + continue; + } + if ch == '\'' { + quote = QuoteState::Single; + i += 1; + continue; + } + if ch == '"' { + quote = QuoteState::Double; + i += 1; + continue; + } + } + } + + if ch != '$' { + i += 1; + continue; + } + + let Some(next) = chars.get(i + 1).copied() else { + i += 1; + continue; + }; + + match next { + '(' => return true, + '{' => { + let mut j = i + 2; + while j < chars.len() && chars[j] != '}' { + j += 1; + } + if j >= chars.len() { + return true; + } + let inner: String = chars[i + 2..j].iter().collect(); + if !is_valid_env_var_name(&inner) + || !allowed_vars.iter().any(|allowed| allowed == &inner) + { + return true; + } + i = j + 1; + continue; + } + c if c.is_ascii_alphabetic() || c == '_' => { + let mut j = i + 2; + while j < chars.len() && (chars[j].is_ascii_alphanumeric() || chars[j] == '_') { + j += 1; + } + let name: String = chars[i + 1..j].iter().collect(); + if !allowed_vars.iter().any(|allowed| allowed == &name) { + return true; + } + i = j; + continue; + } + c if c.is_ascii_digit() || matches!(c, '#' | '?' | '!' | '$' | '*' | '@' | '-') => { + return true; + } + _ => {} + } + + i += 1; + } + + false +} + +fn strip_wrapping_quotes(token: &str) -> &str { + token.trim_matches(|c| c == '"' || c == '\'') +} + +fn looks_like_path(candidate: &str) -> bool { + candidate.starts_with('/') + || candidate.starts_with("./") + || candidate.starts_with("../") + || candidate.starts_with('~') + || candidate == "." + || candidate == ".." + || candidate.contains('/') +} + +fn is_allowlist_entry_match(allowed: &str, executable: &str, executable_base: &str) -> bool { + let allowed = strip_wrapping_quotes(allowed).trim(); + if allowed.is_empty() { + return false; + } + if allowed == "*" { + return true; + } + if looks_like_path(allowed) { + let allowed_path = expand_user_path(allowed); + let executable_path = expand_user_path(executable); + return executable_path == allowed_path; + } + allowed == executable_base +} + +// ── SecurityPolicy Methods ────────────────────────────────────────────────── + +impl SecurityPolicy { + /// Classify command risk. Any high-risk segment marks the whole command high. + pub fn command_risk_level(&self, command: &str) -> CommandRiskLevel { + let mut saw_medium = false; + + for segment in split_unquoted_segments(command) { + let cmd_part = skip_env_assignments(&segment); + let mut words = cmd_part.split_whitespace(); + let Some(base_raw) = words.next() else { + continue; + }; + + let base = base_raw + .rsplit('/') + .next() + .unwrap_or("") + .to_ascii_lowercase(); + + let args: Vec = words.map(|w| w.to_ascii_lowercase()).collect(); + let joined_segment = cmd_part.to_ascii_lowercase(); + + if matches!( + base.as_str(), + "rm" | "mkfs" + | "dd" + | "shutdown" + | "reboot" + | "halt" + | "poweroff" + | "sudo" + | "su" + | "chown" + | "chmod" + | "useradd" + | "userdel" + | "usermod" + | "passwd" + | "mount" + | "umount" + | "iptables" + | "ufw" + | "firewall-cmd" + | "curl" + | "wget" + | "nc" + | "ncat" + | "netcat" + | "scp" + | "ssh" + | "ftp" + | "telnet" + ) { + return CommandRiskLevel::High; + } + + if joined_segment.contains("rm -rf /") + || joined_segment.contains("rm -fr /") + || joined_segment.contains(":(){:|:&};:") + { + return CommandRiskLevel::High; + } + + let medium = match base.as_str() { + "git" => args.first().is_some_and(|verb| { + matches!( + verb.as_str(), + "commit" + | "push" + | "reset" + | "clean" + | "rebase" + | "merge" + | "cherry-pick" + | "revert" + | "branch" + | "checkout" + | "switch" + | "tag" + ) + }), + "npm" | "pnpm" | "yarn" => args.first().is_some_and(|verb| { + matches!( + verb.as_str(), + "install" | "add" | "remove" | "uninstall" | "update" | "publish" + ) + }), + "cargo" => args.first().is_some_and(|verb| { + matches!( + verb.as_str(), + "add" | "remove" | "install" | "clean" | "publish" + ) + }), + "touch" | "mkdir" | "mv" | "cp" | "ln" => true, + _ => false, + }; + + saw_medium |= medium; + } + + if saw_medium { + CommandRiskLevel::Medium + } else { + CommandRiskLevel::Low + } + } + + /// Validate full command execution policy (allowlist + risk gate). + pub fn validate_command_execution( + &self, + command: &str, + approved: bool, + ) -> Result { + if !self.is_command_allowed(command) { + return Err(format!("Command not allowed by security policy: {command}")); + } + + let risk = self.command_risk_level(command); + + if risk == CommandRiskLevel::High { + if self.block_high_risk_commands { + return Err("Command blocked: high-risk command is disallowed by policy".into()); + } + if self.autonomy == AutonomyLevel::Supervised && !approved { + return Err( + "Command requires explicit approval (approved=true): high-risk operation" + .into(), + ); + } + } + + if risk == CommandRiskLevel::Medium + && self.autonomy == AutonomyLevel::Supervised + && self.require_approval_for_medium_risk + && !approved + { + return Err( + "Command requires explicit approval (approved=true): medium-risk operation".into(), + ); + } + + Ok(risk) + } + + /// Check if a shell command is allowed. + pub fn is_command_allowed(&self, command: &str) -> bool { + if self.autonomy == AutonomyLevel::ReadOnly { + return false; + } + + if command.contains('`') + || contains_disallowed_unquoted_shell_variable_expansion( + command, + &self.shell_env_passthrough, + ) + || command.contains("<(") + || command.contains(">(") + { + return false; + } + + if contains_unquoted_char(command, '>') || contains_unquoted_char(command, '<') { + return false; + } + + if command + .split_whitespace() + .any(|w| w == "tee" || w.ends_with("/tee")) + { + return false; + } + + if contains_unquoted_single_ampersand(command) { + return false; + } + + let segments = split_unquoted_segments(command); + for segment in &segments { + let cmd_part = skip_env_assignments(segment); + let mut words = cmd_part.split_whitespace(); + let executable = strip_wrapping_quotes(words.next().unwrap_or("")).trim(); + let base_cmd = executable.rsplit('/').next().unwrap_or(""); + + if base_cmd.is_empty() { + continue; + } + + if !self + .allowed_commands + .iter() + .any(|allowed| is_allowlist_entry_match(allowed, executable, base_cmd)) + { + return false; + } + + let args: Vec = words.map(|w| w.to_ascii_lowercase()).collect(); + if !self.is_args_safe(base_cmd, &args) { + return false; + } + } + + segments.iter().any(|s| { + let s = skip_env_assignments(s.trim()); + s.split_whitespace().next().is_some_and(|w| !w.is_empty()) + }) + } + + /// Check for dangerous arguments that allow sub-command execution. + fn is_args_safe(&self, base: &str, args: &[String]) -> bool { + let base = base.to_ascii_lowercase(); + match base.as_str() { + "find" => !args.iter().any(|arg| arg == "-exec" || arg == "-ok"), + "git" => !args.iter().any(|arg| { + arg == "config" + || arg.starts_with("config.") + || arg == "alias" + || arg.starts_with("alias.") + || arg == "-c" + }), + _ => true, + } + } + + /// Return the first path-like argument blocked by path policy. + pub fn forbidden_path_argument(&self, command: &str) -> Option { + let forbidden_candidate = |raw: &str| { + let candidate = strip_wrapping_quotes(raw).trim(); + if candidate.is_empty() || candidate.contains("://") { + return None; + } + if looks_like_path(candidate) && !self.is_path_allowed(candidate) { + Some(candidate.to_string()) + } else { + None + } + }; + + for segment in split_unquoted_segments(command) { + let cmd_part = skip_env_assignments(&segment); + let mut words = cmd_part.split_whitespace(); + let Some(_executable) = words.next() else { + continue; + }; + + for token in words { + let candidate = strip_wrapping_quotes(token).trim(); + if candidate.is_empty() || candidate.contains("://") { + continue; + } + + if candidate.starts_with('-') { + if let Some((_, value)) = candidate.split_once('=') { + let blocked = forbidden_candidate(value); + if blocked.is_some() { + return blocked; + } + } + continue; + } + + if let Some(blocked) = forbidden_candidate(candidate) { + return Some(blocked); + } + } + } + + None + } + + /// Check if a file path is allowed (no path traversal, within workspace). + pub fn is_path_allowed(&self, path: &str) -> bool { + if path.contains('\0') { + return false; + } + + if Path::new(path) + .components() + .any(|c| matches!(c, std::path::Component::ParentDir)) + { + return false; + } + + let lower = path.to_lowercase(); + if lower.contains("..%2f") || lower.contains("%2f..") { + return false; + } + + if path.starts_with('~') && path != "~" && !path.starts_with("~/") { + return false; + } + + let expanded_path = expand_user_path(path); + + if self.workspace_only && expanded_path.is_absolute() { + return false; + } + + for forbidden in &self.forbidden_paths { + let forbidden_path = expand_user_path(forbidden); + if expanded_path.starts_with(forbidden_path) { + return false; + } + } + + true + } + + /// Validate that a resolved path is inside the workspace or an allowed root. + pub fn is_resolved_path_allowed(&self, resolved: &Path) -> bool { + let workspace_root = self + .workspace_dir + .canonicalize() + .unwrap_or_else(|_| self.workspace_dir.clone()); + if resolved.starts_with(&workspace_root) { + return true; + } + + for root in &self.allowed_roots { + let canonical = root.canonicalize().unwrap_or_else(|_| root.clone()); + if resolved.starts_with(&canonical) { + return true; + } + } + + for forbidden in &self.forbidden_paths { + let forbidden_path = expand_user_path(forbidden); + if resolved.starts_with(&forbidden_path) { + return false; + } + } + + if !self.workspace_only { + return true; + } + + false + } + + /// Returns human-readable guidance on how to fix path violations. + pub fn resolved_path_violation_message(&self, resolved: &Path) -> String { + let guidance = if self.allowed_roots.is_empty() { + "Add the directory to allowed_roots, or move the file into the workspace." + } else { + "Add a matching parent directory to allowed_roots, or move the file into the workspace." + }; + format!( + "Resolved path escapes workspace allowlist: {}. {}", + resolved.display(), + guidance + ) + } + + /// Check if autonomy level permits any action at all. + pub fn can_act(&self) -> bool { + self.autonomy != AutonomyLevel::ReadOnly + } + + /// Enforce policy for a tool operation. + pub fn enforce_tool_operation( + &self, + operation: ToolOperation, + operation_name: &str, + ) -> Result<(), String> { + match operation { + ToolOperation::Read => Ok(()), + ToolOperation::Act => { + if !self.can_act() { + return Err(format!( + "Security policy: read-only mode, cannot perform '{operation_name}'" + )); + } + if !self.record_action() { + return Err("Rate limit exceeded: action budget exhausted".to_string()); + } + Ok(()) + } + } + } + + /// Record an action and check if the rate limit has been exceeded. + pub fn record_action(&self) -> bool { + let count = self.tracker.record(); + count <= self.max_actions_per_hour as usize + } + + /// Check if the rate limit would be exceeded without recording. + pub fn is_rate_limited(&self) -> bool { + self.tracker.count() >= self.max_actions_per_hour as usize + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn test_policy() -> SecurityPolicy { + SecurityPolicy::default() + } + + // ── AutonomyLevel tests ───────────────────────────────────────────── + + #[test] + fn default_autonomy_is_supervised() { + assert_eq!(AutonomyLevel::default(), AutonomyLevel::Supervised); + } + + #[test] + fn can_act_readonly_is_false() { + let p = SecurityPolicy { + autonomy: AutonomyLevel::ReadOnly, + ..SecurityPolicy::default() + }; + assert!(!p.can_act()); + } + + #[test] + fn can_act_supervised_is_true() { + assert!(test_policy().can_act()); + } + + #[test] + fn can_act_full_is_true() { + let p = SecurityPolicy { + autonomy: AutonomyLevel::Full, + ..SecurityPolicy::default() + }; + assert!(p.can_act()); + } + + // ── Path validation tests ─────────────────────────────────────────── + + #[test] + fn path_relative_allowed() { + assert!(test_policy().is_path_allowed("src/main.rs")); + } + + #[test] + fn path_traversal_blocked() { + assert!(!test_policy().is_path_allowed("../../../etc/passwd")); + } + + #[test] + fn path_absolute_blocked_workspace_only() { + assert!(!test_policy().is_path_allowed("/tmp/file.txt")); + } + + #[test] + fn path_absolute_allowed_workspace_not_only() { + let p = SecurityPolicy { + workspace_only: false, + ..SecurityPolicy::default() + }; + // /some/safe/path not in forbidden list + assert!(p.is_path_allowed("/some/safe/path")); + } + + #[test] + fn path_forbidden_blocked() { + let p = SecurityPolicy { + workspace_only: false, + ..SecurityPolicy::default() + }; + assert!(!p.is_path_allowed("/etc/passwd")); + } + + #[test] + fn path_null_byte_blocked() { + assert!(!test_policy().is_path_allowed("file\0.txt")); + } + + #[test] + fn path_url_encoded_traversal_blocked() { + assert!(!test_policy().is_path_allowed("..%2f..%2fetc/passwd")); + } + + #[test] + fn path_tilde_user_blocked() { + assert!(!test_policy().is_path_allowed("~root/.ssh/id_rsa")); + } + + #[test] + fn path_dotfile_in_workspace_allowed() { + assert!(test_policy().is_path_allowed(".env")); + } + + // ── Resolved path tests ───────────────────────────────────────────── + + #[test] + fn resolved_path_inside_workspace() { + let dir = tempfile::tempdir().unwrap(); + let p = SecurityPolicy { + workspace_dir: dir.path().to_path_buf(), + ..SecurityPolicy::default() + }; + let inside = dir.path().join("src/main.rs"); + assert!(p.is_resolved_path_allowed(&inside)); + } + + #[test] + fn resolved_path_outside_workspace_blocked() { + let dir = tempfile::tempdir().unwrap(); + let p = SecurityPolicy { + workspace_dir: dir.path().to_path_buf(), + ..SecurityPolicy::default() + }; + assert!(!p.is_resolved_path_allowed(Path::new("/etc/passwd"))); + } + + #[test] + fn resolved_path_allowed_roots() { + let dir = tempfile::tempdir().unwrap(); + let extra = tempfile::tempdir().unwrap(); + let p = SecurityPolicy { + workspace_dir: dir.path().to_path_buf(), + allowed_roots: vec![extra.path().to_path_buf()], + ..SecurityPolicy::default() + }; + let inside_extra = extra.path().join("file.txt"); + assert!(p.is_resolved_path_allowed(&inside_extra)); + } + + #[cfg(unix)] + #[test] + fn resolved_path_symlink_escape_blocked() { + let workspace = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + let link_path = workspace.path().join("link"); + std::os::unix::fs::symlink(outside.path(), &link_path).unwrap(); + let resolved = link_path.canonicalize().unwrap(); + + let p = SecurityPolicy { + workspace_dir: workspace.path().to_path_buf(), + ..SecurityPolicy::default() + }; + assert!(!p.is_resolved_path_allowed(&resolved)); + } + + // ── Command allowlist tests ───────────────────────────────────────── + + #[test] + fn command_allowed_basic() { + assert!(test_policy().is_command_allowed("ls -la")); + } + + #[test] + fn command_blocked_unknown() { + assert!(!test_policy().is_command_allowed("python3 script.py")); + } + + #[test] + fn command_readonly_blocks_all() { + let p = SecurityPolicy { + autonomy: AutonomyLevel::ReadOnly, + ..SecurityPolicy::default() + }; + assert!(!p.is_command_allowed("ls")); + } + + #[test] + fn command_pipe_both_sides_checked() { + assert!(test_policy().is_command_allowed("grep foo | head -5")); + assert!(!test_policy().is_command_allowed("grep foo | python3")); + } + + #[test] + fn command_semicolon_injection_blocked() { + assert!(!test_policy().is_command_allowed("ls; rm -rf /")); + } + + #[test] + fn command_backtick_injection_blocked() { + assert!(!test_policy().is_command_allowed("echo `rm -rf /`")); + } + + #[test] + fn command_dollar_paren_blocked() { + assert!(!test_policy().is_command_allowed("echo $(rm -rf /)")); + } + + #[test] + fn command_redirect_blocked() { + assert!(!test_policy().is_command_allowed("echo secret > /tmp/file")); + } + + #[test] + fn command_background_blocked() { + assert!(!test_policy().is_command_allowed("ls & rm -rf /")); + } + + #[test] + fn command_and_chain_allowed() { + assert!(test_policy().is_command_allowed("ls && echo ok")); + } + + #[test] + fn command_tee_blocked() { + assert!(!test_policy().is_command_allowed("echo secret | tee /tmp/out")); + } + + #[test] + fn command_find_exec_blocked() { + assert!(!test_policy().is_command_allowed("find . -exec rm {} \\;")); + } + + #[test] + fn command_git_config_blocked() { + assert!(!test_policy().is_command_allowed("git config core.editor vim")); + } + + #[test] + fn command_process_substitution_blocked() { + assert!(!test_policy().is_command_allowed("cat <(echo foo)")); + } + + #[test] + fn command_env_prefix_handled() { + assert!(test_policy().is_command_allowed("FOO=bar ls")); + } + + #[test] + fn command_shell_var_blocked() { + assert!(!test_policy().is_command_allowed("echo $HOME")); + } + + #[test] + fn command_shell_var_passthrough_allowed() { + let p = SecurityPolicy { + shell_env_passthrough: vec!["HOME".into()], + ..SecurityPolicy::default() + }; + assert!(p.is_command_allowed("echo $HOME")); + } + + #[test] + fn command_quoted_operators_safe() { + // Quoted semicolons/operators should be treated as literals + assert!(test_policy().is_command_allowed("echo 'hello; world'")); + } + + // ── Risk classification tests ─────────────────────────────────────── + + #[test] + fn risk_low_for_read_ops() { + let p = test_policy(); + assert_eq!(p.command_risk_level("ls -la"), CommandRiskLevel::Low); + assert_eq!(p.command_risk_level("git status"), CommandRiskLevel::Low); + assert_eq!(p.command_risk_level("cat file.txt"), CommandRiskLevel::Low); + } + + #[test] + fn risk_medium_for_mutating() { + let p = test_policy(); + assert_eq!( + p.command_risk_level("git commit -m 'msg'"), + CommandRiskLevel::Medium + ); + assert_eq!(p.command_risk_level("touch file"), CommandRiskLevel::Medium); + assert_eq!( + p.command_risk_level("npm install"), + CommandRiskLevel::Medium + ); + } + + #[test] + fn risk_high_for_dangerous() { + let p = test_policy(); + assert_eq!(p.command_risk_level("rm -rf /"), CommandRiskLevel::High); + assert_eq!(p.command_risk_level("sudo ls"), CommandRiskLevel::High); + assert_eq!( + p.command_risk_level("curl http://evil.com"), + CommandRiskLevel::High + ); + } + + // ── Command validation tests ──────────────────────────────────────── + + #[test] + fn validate_blocks_high_risk() { + let p = test_policy(); + assert!(p.validate_command_execution("rm file", false).is_err()); + } + + #[test] + fn validate_blocks_medium_risk_without_approval() { + let p = test_policy(); + assert!( + p.validate_command_execution("git commit -m x", false) + .is_err() + ); + } + + #[test] + fn validate_allows_medium_risk_with_approval() { + let p = test_policy(); + let result = p.validate_command_execution("git commit -m x", true); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), CommandRiskLevel::Medium); + } + + #[test] + fn validate_allows_low_risk() { + let p = test_policy(); + let result = p.validate_command_execution("ls -la", false); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), CommandRiskLevel::Low); + } + + #[test] + fn validate_full_autonomy_skips_medium_approval() { + let p = SecurityPolicy { + autonomy: AutonomyLevel::Full, + ..SecurityPolicy::default() + }; + assert!( + p.validate_command_execution("git commit -m x", false) + .is_ok() + ); + } + + // ── Rate limiting tests ───────────────────────────────────────────── + + #[test] + fn rate_limit_zero_budget_blocks() { + let p = SecurityPolicy { + max_actions_per_hour: 0, + ..SecurityPolicy::default() + }; + assert!(!p.record_action()); + } + + #[test] + fn rate_limit_boundary() { + let p = SecurityPolicy { + max_actions_per_hour: 3, + ..SecurityPolicy::default() + }; + assert!(p.record_action()); // 1 + assert!(p.record_action()); // 2 + assert!(p.record_action()); // 3 + assert!(!p.record_action()); // 4 = over limit + } + + #[test] + fn is_rate_limited_no_record() { + let p = SecurityPolicy { + max_actions_per_hour: 1, + ..SecurityPolicy::default() + }; + assert!(!p.is_rate_limited()); + p.record_action(); + assert!(p.is_rate_limited()); + } + + #[test] + fn tracker_clone_independence() { + let p = test_policy(); + p.record_action(); + let p2 = p.clone(); + p.record_action(); + // p has 2 actions, p2 should have 1 + assert_eq!(p.tracker.count(), 2); + assert_eq!(p2.tracker.count(), 1); + } + + // ── Enforce tool operation tests ──────────────────────────────────── + + #[test] + fn enforce_read_always_ok() { + let p = SecurityPolicy { + autonomy: AutonomyLevel::ReadOnly, + ..SecurityPolicy::default() + }; + assert!( + p.enforce_tool_operation(ToolOperation::Read, "read") + .is_ok() + ); + } + + #[test] + fn enforce_act_blocked_readonly() { + let p = SecurityPolicy { + autonomy: AutonomyLevel::ReadOnly, + ..SecurityPolicy::default() + }; + assert!( + p.enforce_tool_operation(ToolOperation::Act, "write") + .is_err() + ); + } + + #[test] + fn enforce_act_rate_limited() { + let p = SecurityPolicy { + max_actions_per_hour: 1, + ..SecurityPolicy::default() + }; + assert!( + p.enforce_tool_operation(ToolOperation::Act, "write") + .is_ok() + ); + assert!( + p.enforce_tool_operation(ToolOperation::Act, "write") + .is_err() + ); + } + + // ── Forbidden path argument tests ─────────────────────────────────── + + #[test] + fn forbidden_path_argument_detects_absolute() { + let p = test_policy(); + assert!(p.forbidden_path_argument("cat /etc/passwd").is_some()); + } + + #[test] + fn forbidden_path_argument_safe_relative() { + let p = test_policy(); + assert!(p.forbidden_path_argument("cat src/main.rs").is_none()); + } + + #[test] + fn forbidden_path_argument_option_value() { + let p = test_policy(); + assert!( + p.forbidden_path_argument("cmd --file=/etc/passwd") + .is_some() + ); + } + + // ── Default policy sanity ─────────────────────────────────────────── + + #[test] + fn default_policy_sanity() { + let p = SecurityPolicy::default(); + assert_eq!(p.autonomy, AutonomyLevel::Supervised); + assert!(p.workspace_only); + assert!(!p.allowed_commands.is_empty()); + assert!(!p.forbidden_paths.is_empty()); + assert!(p.block_high_risk_commands); + assert!(p.require_approval_for_medium_risk); + } + + #[test] + fn security_checklist_root_path_blocked() { + assert!(!test_policy().is_path_allowed("/")); + } + + #[test] + fn security_checklist_all_system_dirs_blocked() { + let p = test_policy(); + for dir in &[ + "/etc", "/root", "/usr", "/bin", "/sbin", "/boot", "/dev", "/proc", "/sys", + ] { + assert!(!p.is_path_allowed(dir), "{dir} should be blocked"); + } + } + + #[test] + fn resolved_path_violation_message_content() { + let p = test_policy(); + let msg = p.resolved_path_violation_message(Path::new("/etc/passwd")); + assert!(msg.contains("escapes workspace")); + assert!(msg.contains("allowed_roots")); + } +} diff --git a/crewforge-rs/src/security/secrets.rs b/crewforge-rs/src/security/secrets.rs index 23f38d1..7f3a5a4 100644 --- a/crewforge-rs/src/security/secrets.rs +++ b/crewforge-rs/src/security/secrets.rs @@ -27,6 +27,7 @@ use std::fs; use std::path::{Path, PathBuf}; /// Length of the random encryption key in bytes (256-bit, matches `ChaCha20`). +#[allow(dead_code)] const KEY_LEN: usize = 32; /// ChaCha20-Poly1305 nonce length in bytes. @@ -254,6 +255,7 @@ fn hex_encode(data: &[u8]) -> String { /// Build the `/grant` argument for `icacls` using a normalized username. /// Returns `None` when the username is empty or whitespace-only. +#[allow(dead_code)] fn build_windows_icacls_grant_arg(username: &str) -> Option { let normalized = username.trim(); if normalized.is_empty() { diff --git a/crewforge-rs/src/tools/content_search.rs b/crewforge-rs/src/tools/content_search.rs new file mode 100644 index 0000000..4333611 --- /dev/null +++ b/crewforge-rs/src/tools/content_search.rs @@ -0,0 +1,775 @@ +use crate::agent::ToolResult; +use crate::security::SecurityPolicy; +use async_trait::async_trait; +use std::process::Stdio; +use std::sync::{Arc, LazyLock}; + +const MAX_RESULTS: usize = 1000; +const MAX_OUTPUT_BYTES: usize = 1_048_576; // 1 MB +const TIMEOUT_SECS: u64 = 30; + +pub struct ContentSearchTool { + security: Arc, + has_rg: bool, +} + +impl ContentSearchTool { + pub fn new(security: Arc) -> Self { + let has_rg = which::which("rg").is_ok(); + Self { security, has_rg } + } + + #[cfg(test)] + fn new_with_backend(security: Arc, has_rg: bool) -> Self { + Self { security, has_rg } + } +} + +#[async_trait] +impl crate::agent::Tool for ContentSearchTool { + fn name(&self) -> &str { + "content_search" + } + + fn description(&self) -> &str { + "Search file contents by regex pattern within the workspace. \ + Uses ripgrep (rg) with grep fallback. \ + Output modes: 'content', 'files_with_matches', 'count'." + } + + fn parameters(&self) -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "pattern": { + "type": "string", + "description": "Regular expression pattern to search for" + }, + "path": { + "type": "string", + "description": "Directory to search in, relative to workspace root. Defaults to '.'", + "default": "." + }, + "output_mode": { + "type": "string", + "description": "Output format: 'content', 'files_with_matches', 'count'", + "enum": ["content", "files_with_matches", "count"], + "default": "content" + }, + "include": { + "type": "string", + "description": "File glob filter, e.g. '*.rs', '*.{ts,tsx}'" + }, + "case_sensitive": { + "type": "boolean", + "description": "Case-sensitive matching. Defaults to true", + "default": true + }, + "context_before": { + "type": "integer", + "description": "Lines of context before each match (content mode only)", + "default": 0 + }, + "context_after": { + "type": "integer", + "description": "Lines of context after each match (content mode only)", + "default": 0 + } + }, + "required": ["pattern"] + }) + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + let pattern = args + .get("pattern") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing 'pattern' parameter"))?; + + if pattern.is_empty() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Empty pattern is not allowed.".into()), + }); + } + + let search_path = args.get("path").and_then(|v| v.as_str()).unwrap_or("."); + + let output_mode = args + .get("output_mode") + .and_then(|v| v.as_str()) + .unwrap_or("content"); + + if !matches!(output_mode, "content" | "files_with_matches" | "count") { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!( + "Invalid output_mode '{output_mode}'. Allowed: content, files_with_matches, count." + )), + }); + } + + let include = args.get("include").and_then(|v| v.as_str()); + + let case_sensitive = args + .get("case_sensitive") + .and_then(|v| v.as_bool()) + .unwrap_or(true); + + #[allow(clippy::cast_possible_truncation)] + let context_before = args + .get("context_before") + .and_then(|v| v.as_u64()) + .unwrap_or(0) as usize; + + #[allow(clippy::cast_possible_truncation)] + let context_after = args + .get("context_after") + .and_then(|v| v.as_u64()) + .unwrap_or(0) as usize; + + if self.security.is_rate_limited() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Rate limit exceeded".into()), + }); + } + + if std::path::Path::new(search_path).is_absolute() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Absolute paths are not allowed. Use a relative path.".into()), + }); + } + + if search_path.contains("../") || search_path.contains("..\\") || search_path == ".." { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Path traversal ('..') is not allowed.".into()), + }); + } + + if !self.security.is_path_allowed(search_path) { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!( + "Path '{search_path}' is not allowed by security policy." + )), + }); + } + + if !self.security.record_action() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Rate limit exceeded: action budget exhausted".into()), + }); + } + + let workspace = &self.security.workspace_dir; + let resolved_path = workspace.join(search_path); + + let resolved_canon = match std::fs::canonicalize(&resolved_path) { + Ok(p) => p, + Err(e) => { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Cannot resolve path '{search_path}': {e}")), + }); + } + }; + + if !self.security.is_resolved_path_allowed(&resolved_canon) { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!( + "Resolved path for '{search_path}' is outside the allowed workspace." + )), + }); + } + + let mut cmd = if self.has_rg { + build_rg_command( + pattern, + &resolved_canon, + output_mode, + include, + case_sensitive, + context_before, + context_after, + ) + } else { + build_grep_command( + pattern, + &resolved_canon, + output_mode, + include, + case_sensitive, + context_before, + context_after, + ) + }; + + // Clear environment, keep only safe variables + cmd.env_clear(); + for key in &["PATH", "HOME", "LANG", "LC_ALL", "LC_CTYPE"] { + if let Ok(val) = std::env::var(key) { + cmd.env(key, val); + } + } + + let output = + match tokio::time::timeout(std::time::Duration::from_secs(TIMEOUT_SECS), cmd.output()) + .await + { + Ok(Ok(out)) => out, + Ok(Err(e)) => { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Failed to execute search command: {e}")), + }); + } + Err(_) => { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Search timed out after {TIMEOUT_SECS} seconds.")), + }); + } + }; + + // Exit code: 0 = matches found, 1 = no matches (grep/rg), 2 = error + let exit_code = output.status.code().unwrap_or(-1); + if exit_code >= 2 { + let stderr = String::from_utf8_lossy(&output.stderr); + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Search error: {}", stderr.trim())), + }); + } + + let raw_stdout = String::from_utf8_lossy(&output.stdout); + + let workspace_canon = + std::fs::canonicalize(workspace).unwrap_or_else(|_| workspace.clone()); + + let formatted = format_line_output(&raw_stdout, &workspace_canon, output_mode, MAX_RESULTS); + + // Truncate if too large + let final_output = if formatted.len() > MAX_OUTPUT_BYTES { + let mut truncated = truncate_utf8(&formatted, MAX_OUTPUT_BYTES).to_string(); + truncated.push_str("\n\n[Output truncated: exceeded 1 MB limit]"); + truncated + } else { + formatted + }; + + Ok(ToolResult { + success: true, + output: final_output, + error: None, + }) + } +} + +fn build_rg_command( + pattern: &str, + search_path: &std::path::Path, + output_mode: &str, + include: Option<&str>, + case_sensitive: bool, + context_before: usize, + context_after: usize, +) -> tokio::process::Command { + let mut cmd = tokio::process::Command::new("rg"); + + cmd.arg("--no-heading"); + cmd.arg("--line-number"); + cmd.arg("--with-filename"); + cmd.stdout(Stdio::piped()); + cmd.stderr(Stdio::piped()); + + match output_mode { + "files_with_matches" => { + cmd.arg("--files-with-matches"); + } + "count" => { + cmd.arg("--count"); + } + _ => { + if context_before > 0 { + cmd.arg("-B").arg(context_before.to_string()); + } + if context_after > 0 { + cmd.arg("-A").arg(context_after.to_string()); + } + } + } + + if !case_sensitive { + cmd.arg("-i"); + } + + if let Some(glob) = include { + cmd.arg("--glob").arg(glob); + } + + cmd.arg("--"); + cmd.arg(pattern); + cmd.arg(search_path); + + cmd +} + +fn build_grep_command( + pattern: &str, + search_path: &std::path::Path, + output_mode: &str, + include: Option<&str>, + case_sensitive: bool, + context_before: usize, + context_after: usize, +) -> tokio::process::Command { + let mut cmd = tokio::process::Command::new("grep"); + + cmd.arg("-r"); + cmd.arg("-n"); + cmd.arg("-E"); + cmd.arg("--binary-files=without-match"); + cmd.stdout(Stdio::piped()); + cmd.stderr(Stdio::piped()); + + match output_mode { + "files_with_matches" => { + cmd.arg("-l"); + } + "count" => { + cmd.arg("-c"); + } + _ => { + if context_before > 0 { + cmd.arg("-B").arg(context_before.to_string()); + } + if context_after > 0 { + cmd.arg("-A").arg(context_after.to_string()); + } + } + } + + if !case_sensitive { + cmd.arg("-i"); + } + + if let Some(glob) = include { + cmd.arg("--include").arg(glob); + } + + cmd.arg("--"); + cmd.arg(pattern); + cmd.arg(search_path); + + cmd +} + +fn relativize_path(line: &str, workspace_prefix: &str) -> String { + if let Some(rest) = line.strip_prefix(workspace_prefix) { + let trimmed = rest + .strip_prefix('/') + .or_else(|| rest.strip_prefix('\\')) + .unwrap_or(rest); + return trimmed.to_string(); + } + line.to_string() +} + +fn parse_content_line(line: &str) -> Option<(&str, bool)> { + static MATCH_RE: LazyLock = LazyLock::new(|| { + regex::Regex::new(r"^(?P.+?):\d+:").expect("match line regex must be valid") + }); + static CONTEXT_RE: LazyLock = LazyLock::new(|| { + regex::Regex::new(r"^(?P.+?)-\d+-").expect("context line regex must be valid") + }); + + if let Some(caps) = MATCH_RE.captures(line) { + return caps.name("path").map(|m| (m.as_str(), true)); + } + + if let Some(caps) = CONTEXT_RE.captures(line) { + return caps.name("path").map(|m| (m.as_str(), false)); + } + + None +} + +fn parse_count_line(line: &str) -> Option<(&str, usize)> { + static COUNT_RE: LazyLock = LazyLock::new(|| { + regex::Regex::new(r"^(?P.+?):(?P\d+)\s*$").expect("count line regex valid") + }); + + let caps = COUNT_RE.captures(line)?; + let path = caps.name("path")?.as_str(); + let count = caps.name("count")?.as_str().parse::().ok()?; + Some((path, count)) +} + +fn format_line_output( + raw: &str, + workspace_canon: &std::path::Path, + output_mode: &str, + max_results: usize, +) -> String { + if raw.trim().is_empty() { + return "No matches found.".to_string(); + } + + let workspace_prefix = workspace_canon.to_string_lossy(); + + let mut lines: Vec = Vec::new(); + let mut truncated = false; + let mut file_set = std::collections::HashSet::new(); + let mut total_matches: usize = 0; + + for line in raw.lines() { + if line.is_empty() { + continue; + } + + let relativized = relativize_path(line, &workspace_prefix); + + match output_mode { + "files_with_matches" => { + let path = relativized.trim(); + if !path.is_empty() && file_set.insert(path.to_string()) { + lines.push(path.to_string()); + if lines.len() >= max_results { + truncated = true; + break; + } + } + } + "count" => { + if let Some((path, count)) = parse_count_line(&relativized) + && count > 0 + { + file_set.insert(path.to_string()); + total_matches += count; + lines.push(format!("{path}:{count}")); + if lines.len() >= max_results { + truncated = true; + break; + } + } + } + _ => { + if relativized == "--" { + lines.push(relativized); + if lines.len() >= max_results { + truncated = true; + break; + } + continue; + } + if let Some((path, is_match)) = parse_content_line(&relativized) { + file_set.insert(path.to_string()); + if is_match { + total_matches += 1; + } + } else { + total_matches += 1; + } + lines.push(relativized); + if lines.len() >= max_results { + truncated = true; + break; + } + } + } + } + + if lines.is_empty() { + return "No matches found.".to_string(); + } + + use std::fmt::Write; + let mut buf = lines.join("\n"); + + if truncated { + let _ = write!( + buf, + "\n\n[Results truncated: showing first {max_results} results]" + ); + } + + match output_mode { + "files_with_matches" => { + let _ = write!(buf, "\n\nTotal: {} files", file_set.len()); + } + "count" => { + let _ = write!( + buf, + "\n\nTotal: {} matches in {} files", + total_matches, + file_set.len() + ); + } + _ => { + let _ = write!( + buf, + "\n\nTotal: {} matching lines in {} files", + total_matches, + file_set.len() + ); + } + } + + buf +} + +fn truncate_utf8(input: &str, max_bytes: usize) -> &str { + if input.len() <= max_bytes { + return input; + } + let mut end = max_bytes; + while end > 0 && !input.is_char_boundary(end) { + end -= 1; + } + &input[..end] +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::agent::Tool; + use serde_json::json; + + fn test_security(workspace: std::path::PathBuf) -> Arc { + Arc::new(SecurityPolicy { + workspace_dir: workspace, + ..SecurityPolicy::default() + }) + } + + fn create_test_files(dir: &tempfile::TempDir) { + std::fs::write( + dir.path().join("hello.rs"), + "fn main() {\n println!(\"hello\");\n}\n", + ) + .unwrap(); + std::fs::write( + dir.path().join("lib.rs"), + "pub fn greet() {\n println!(\"greet\");\n}\n", + ) + .unwrap(); + std::fs::write(dir.path().join("readme.txt"), "This is a readme file.\n").unwrap(); + } + + #[tokio::test] + async fn content_search_basic_match() { + let dir = tempfile::tempdir().unwrap(); + create_test_files(&dir); + + let tool = ContentSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"pattern": "fn main"})).await.unwrap(); + + assert!(result.success); + assert!(result.output.contains("hello.rs")); + assert!(result.output.contains("fn main")); + } + + #[tokio::test] + async fn content_search_files_with_matches_mode() { + let dir = tempfile::tempdir().unwrap(); + create_test_files(&dir); + + let tool = ContentSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"pattern": "println", "output_mode": "files_with_matches"})) + .await + .unwrap(); + + assert!(result.success); + assert!(result.output.contains("hello.rs")); + assert!(result.output.contains("lib.rs")); + assert!(!result.output.contains("readme.txt")); + assert!(result.output.contains("Total: 2 files")); + } + + #[tokio::test] + async fn content_search_count_mode() { + let dir = tempfile::tempdir().unwrap(); + create_test_files(&dir); + + let tool = ContentSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"pattern": "println", "output_mode": "count"})) + .await + .unwrap(); + + assert!(result.success); + assert!(result.output.contains("hello.rs")); + assert!(result.output.contains("lib.rs")); + assert!(result.output.contains("Total:")); + } + + #[tokio::test] + async fn content_search_case_insensitive() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("test.txt"), "Hello World\nhello world\n").unwrap(); + + let tool = ContentSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"pattern": "HELLO", "case_sensitive": false})) + .await + .unwrap(); + + assert!(result.success); + assert!(result.output.contains("Hello World")); + assert!(result.output.contains("hello world")); + } + + #[tokio::test] + async fn content_search_include_filter() { + let dir = tempfile::tempdir().unwrap(); + create_test_files(&dir); + + let tool = ContentSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"pattern": "fn", "include": "*.rs"})) + .await + .unwrap(); + + assert!(result.success); + assert!(result.output.contains("hello.rs")); + assert!(!result.output.contains("readme.txt")); + } + + #[tokio::test] + async fn content_search_context_lines() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write( + dir.path().join("ctx.rs"), + "line1\nline2\ntarget_line\nline4\nline5\n", + ) + .unwrap(); + + let tool = ContentSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"pattern": "target_line", "context_before": 1, "context_after": 1})) + .await + .unwrap(); + + assert!(result.success); + assert!(result.output.contains("target_line")); + assert!(result.output.contains("line2")); + assert!(result.output.contains("line4")); + } + + #[tokio::test] + async fn content_search_no_matches() { + let dir = tempfile::tempdir().unwrap(); + create_test_files(&dir); + + let tool = ContentSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"pattern": "nonexistent_string_xyz"})) + .await + .unwrap(); + + assert!(result.success); + assert!(result.output.contains("No matches found")); + } + + #[tokio::test] + async fn content_search_empty_pattern_rejected() { + let tool = ContentSearchTool::new(test_security(std::env::temp_dir())); + let result = tool.execute(json!({"pattern": ""})).await.unwrap(); + + assert!(!result.success); + assert!(result.error.as_ref().unwrap().contains("Empty pattern")); + } + + #[tokio::test] + async fn content_search_rejects_absolute_path() { + let tool = ContentSearchTool::new(test_security(std::env::temp_dir())); + let result = tool + .execute(json!({"pattern": "test", "path": "/etc"})) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.as_ref().unwrap().contains("Absolute paths")); + } + + #[tokio::test] + async fn content_search_rejects_path_traversal() { + let tool = ContentSearchTool::new(test_security(std::env::temp_dir())); + let result = tool + .execute(json!({"pattern": "test", "path": "../../../etc"})) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.as_ref().unwrap().contains("Path traversal")); + } + + #[tokio::test] + async fn content_search_rate_limited() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("file.txt"), "test content\n").unwrap(); + + let security = Arc::new(SecurityPolicy { + workspace_dir: dir.path().to_path_buf(), + max_actions_per_hour: 0, + ..SecurityPolicy::default() + }); + let tool = ContentSearchTool::new(security); + let result = tool.execute(json!({"pattern": "test"})).await.unwrap(); + + assert!(!result.success); + assert!(result.error.as_ref().unwrap().contains("Rate limit")); + } + + #[tokio::test] + async fn content_search_multiline_without_rg() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("test.txt"), "line1\nline2\n").unwrap(); + + let tool = ContentSearchTool::new_with_backend( + test_security(dir.path().to_path_buf()), + false, // no rg + ); + // Without multiline support in grep fallback, this should still work for basic patterns + let result = tool.execute(json!({"pattern": "line1"})).await.unwrap(); + + assert!(result.success); + } + + #[test] + fn relativize_path_strips_prefix() { + let result = relativize_path("/workspace/src/main.rs:42:fn main()", "/workspace"); + assert_eq!(result, "src/main.rs:42:fn main()"); + } + + #[test] + fn relativize_path_no_prefix() { + let result = relativize_path("src/main.rs:42:fn main()", "/workspace"); + assert_eq!(result, "src/main.rs:42:fn main()"); + } + + #[test] + fn truncate_utf8_keeps_char_boundary() { + let text = "abc\u{4f60}\u{597d}"; // "abc你好" + let truncated = truncate_utf8(text, 4); + assert_eq!(truncated, "abc"); + } +} diff --git a/crewforge-rs/src/tools/file_edit.rs b/crewforge-rs/src/tools/file_edit.rs new file mode 100644 index 0000000..16a73a8 --- /dev/null +++ b/crewforge-rs/src/tools/file_edit.rs @@ -0,0 +1,485 @@ +use crate::agent::ToolResult; +use crate::security::SecurityPolicy; +use async_trait::async_trait; +use std::sync::Arc; + +pub struct FileEditTool { + security: Arc, +} + +impl FileEditTool { + pub fn new(security: Arc) -> Self { + Self { security } + } +} + +#[async_trait] +impl crate::agent::Tool for FileEditTool { + fn name(&self) -> &str { + "file_edit" + } + + fn description(&self) -> &str { + "Edit a file by replacing an exact string match with new content" + } + + fn parameters(&self) -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "path": { + "type": "string", + "description": "Relative path to the file from workspace root" + }, + "old_string": { + "type": "string", + "description": "The exact text to find and replace (must appear exactly once)" + }, + "new_string": { + "type": "string", + "description": "The replacement text (empty string to delete the matched text)" + } + }, + "required": ["path", "old_string", "new_string"] + }) + } + + fn is_mutating(&self) -> bool { + true + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + let path = args + .get("path") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing 'path' parameter"))?; + let old_string = args + .get("old_string") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing 'old_string' parameter"))?; + let new_string = args + .get("new_string") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing 'new_string' parameter"))?; + + if old_string.is_empty() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("old_string must not be empty".into()), + }); + } + + if !self.security.can_act() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Action blocked: autonomy is read-only".into()), + }); + } + + if self.security.is_rate_limited() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Rate limit exceeded".into()), + }); + } + + if !self.security.is_path_allowed(path) { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Path not allowed by security policy: {path}")), + }); + } + + let full_path = self.security.workspace_dir.join(path); + + let Some(parent) = full_path.parent() else { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Invalid path: missing parent directory".into()), + }); + }; + + let resolved_parent = match tokio::fs::canonicalize(parent).await { + Ok(p) => p, + Err(e) => { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Failed to resolve file path: {e}")), + }); + } + }; + + if !self.security.is_resolved_path_allowed(&resolved_parent) { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some( + self.security + .resolved_path_violation_message(&resolved_parent), + ), + }); + } + + let Some(file_name) = full_path.file_name() else { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Invalid path: missing file name".into()), + }); + }; + + let resolved_target = resolved_parent.join(file_name); + + // Symlink check + if let Ok(meta) = tokio::fs::symlink_metadata(&resolved_target).await + && meta.file_type().is_symlink() + { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!( + "Refusing to edit through symlink: {}", + resolved_target.display() + )), + }); + } + + if !self.security.record_action() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Rate limit exceeded: action budget exhausted".into()), + }); + } + + let content = match tokio::fs::read_to_string(&resolved_target).await { + Ok(c) => c, + Err(e) => { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Failed to read file: {e}")), + }); + } + }; + + let match_count = content.matches(old_string).count(); + + if match_count == 0 { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("old_string not found in file".into()), + }); + } + + if match_count > 1 { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!( + "old_string matches {match_count} times; must match exactly once" + )), + }); + } + + let new_content = content.replacen(old_string, new_string, 1); + + match tokio::fs::write(&resolved_target, &new_content).await { + Ok(()) => Ok(ToolResult { + success: true, + output: format!( + "Edited {path}: replaced 1 occurrence ({} bytes)", + new_content.len() + ), + error: None, + }), + Err(e) => Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Failed to write file: {e}")), + }), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::agent::Tool; + use crate::security::AutonomyLevel; + use serde_json::json; + + fn test_security(workspace: std::path::PathBuf) -> Arc { + Arc::new(SecurityPolicy { + workspace_dir: workspace, + ..SecurityPolicy::default() + }) + } + + #[tokio::test] + async fn file_edit_replaces_single_match() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("test.txt"), "hello world") + .await + .unwrap(); + + let tool = FileEditTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({ + "path": "test.txt", + "old_string": "hello", + "new_string": "goodbye" + })) + .await + .unwrap(); + + assert!(result.success, "edit should succeed: {:?}", result.error); + assert!(result.output.contains("replaced 1 occurrence")); + + let content = tokio::fs::read_to_string(dir.path().join("test.txt")) + .await + .unwrap(); + assert_eq!(content, "goodbye world"); + } + + #[tokio::test] + async fn file_edit_not_found() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("test.txt"), "hello world") + .await + .unwrap(); + + let tool = FileEditTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({ + "path": "test.txt", + "old_string": "nonexistent", + "new_string": "replacement" + })) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.as_deref().unwrap_or("").contains("not found")); + } + + #[tokio::test] + async fn file_edit_multiple_matches() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("test.txt"), "aaa bbb aaa") + .await + .unwrap(); + + let tool = FileEditTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({ + "path": "test.txt", + "old_string": "aaa", + "new_string": "ccc" + })) + .await + .unwrap(); + + assert!(!result.success); + assert!( + result + .error + .as_deref() + .unwrap_or("") + .contains("matches 2 times") + ); + } + + #[tokio::test] + async fn file_edit_delete_via_empty_new_string() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("test.txt"), "keep remove keep") + .await + .unwrap(); + + let tool = FileEditTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({ + "path": "test.txt", + "old_string": " remove", + "new_string": "" + })) + .await + .unwrap(); + + assert!(result.success); + let content = tokio::fs::read_to_string(dir.path().join("test.txt")) + .await + .unwrap(); + assert_eq!(content, "keep keep"); + } + + #[tokio::test] + async fn file_edit_rejects_empty_old_string() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("test.txt"), "hello") + .await + .unwrap(); + + let tool = FileEditTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({ + "path": "test.txt", + "old_string": "", + "new_string": "x" + })) + .await + .unwrap(); + + assert!(!result.success); + assert!( + result + .error + .as_deref() + .unwrap_or("") + .contains("must not be empty") + ); + } + + #[tokio::test] + async fn file_edit_blocks_path_traversal() { + let dir = tempfile::tempdir().unwrap(); + let tool = FileEditTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({ + "path": "../../etc/passwd", + "old_string": "root", + "new_string": "hacked" + })) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.as_ref().unwrap().contains("not allowed")); + } + + #[tokio::test] + async fn file_edit_blocks_readonly_mode() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("test.txt"), "hello") + .await + .unwrap(); + + let security = Arc::new(SecurityPolicy { + autonomy: AutonomyLevel::ReadOnly, + workspace_dir: dir.path().to_path_buf(), + ..SecurityPolicy::default() + }); + let tool = FileEditTool::new(security); + let result = tool + .execute(json!({ + "path": "test.txt", + "old_string": "hello", + "new_string": "world" + })) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.as_deref().unwrap_or("").contains("read-only")); + } + + #[cfg(unix)] + #[tokio::test] + async fn file_edit_blocks_symlink_escape() { + let workspace = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + + std::os::unix::fs::symlink(outside.path(), workspace.path().join("escape_dir")).unwrap(); + + let tool = FileEditTool::new(test_security(workspace.path().to_path_buf())); + let result = tool + .execute(json!({ + "path": "escape_dir/target.txt", + "old_string": "a", + "new_string": "b" + })) + .await + .unwrap(); + + assert!(!result.success); + } + + #[cfg(unix)] + #[tokio::test] + async fn file_edit_blocks_symlink_target_file() { + let workspace = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + tokio::fs::write(outside.path().join("target.txt"), "original") + .await + .unwrap(); + std::os::unix::fs::symlink( + outside.path().join("target.txt"), + workspace.path().join("linked.txt"), + ) + .unwrap(); + + let tool = FileEditTool::new(test_security(workspace.path().to_path_buf())); + let result = tool + .execute(json!({ + "path": "linked.txt", + "old_string": "original", + "new_string": "hacked" + })) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.as_deref().unwrap_or("").contains("symlink")); + + let content = tokio::fs::read_to_string(outside.path().join("target.txt")) + .await + .unwrap(); + assert_eq!(content, "original"); + } + + #[tokio::test] + async fn file_edit_blocks_null_byte_in_path() { + let dir = tempfile::tempdir().unwrap(); + let tool = FileEditTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({ + "path": "test\0evil.txt", + "old_string": "old", + "new_string": "new" + })) + .await + .unwrap(); + assert!(!result.success); + } + + #[tokio::test] + async fn file_edit_nonexistent_file() { + let dir = tempfile::tempdir().unwrap(); + let tool = FileEditTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({ + "path": "missing.txt", + "old_string": "a", + "new_string": "b" + })) + .await + .unwrap(); + + assert!(!result.success); + assert!( + result + .error + .as_deref() + .unwrap_or("") + .contains("Failed to read file") + ); + } +} diff --git a/crewforge-rs/src/tools/file_read.rs b/crewforge-rs/src/tools/file_read.rs new file mode 100644 index 0000000..6c62e5b --- /dev/null +++ b/crewforge-rs/src/tools/file_read.rs @@ -0,0 +1,388 @@ +use crate::agent::ToolResult; +use crate::security::SecurityPolicy; +use async_trait::async_trait; +use std::sync::Arc; + +const MAX_FILE_SIZE_BYTES: u64 = 10 * 1024 * 1024; + +pub struct FileReadTool { + security: Arc, +} + +impl FileReadTool { + pub fn new(security: Arc) -> Self { + Self { security } + } +} + +#[async_trait] +impl crate::agent::Tool for FileReadTool { + fn name(&self) -> &str { + "file_read" + } + + fn description(&self) -> &str { + "Read file contents with line numbers. Supports partial reading via offset and limit." + } + + fn parameters(&self) -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "path": { + "type": "string", + "description": "Relative path to the file from workspace root" + }, + "offset": { + "type": "integer", + "description": "Starting line number (1-based, default: 1)" + }, + "limit": { + "type": "integer", + "description": "Maximum number of lines to return (default: all)" + } + }, + "required": ["path"] + }) + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + let path = args + .get("path") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing 'path' parameter"))?; + + if self.security.is_rate_limited() { + return Ok(ToolResult::denied("Rate limit exceeded")); + } + + if !self.security.is_path_allowed(path) { + return Ok(ToolResult::denied(format!( + "Path not allowed by security policy: {path}" + ))); + } + + let full_path = self.security.workspace_dir.join(path); + + let resolved_path = match tokio::fs::canonicalize(&full_path).await { + Ok(p) => p, + Err(e) => { + return Ok(ToolResult::denied(format!( + "Failed to resolve file path: {e}" + ))); + } + }; + + if !self.security.is_resolved_path_allowed(&resolved_path) { + return Ok(ToolResult::denied( + self.security + .resolved_path_violation_message(&resolved_path), + )); + } + + match tokio::fs::metadata(&resolved_path).await { + Ok(meta) => { + if meta.len() > MAX_FILE_SIZE_BYTES { + return Ok(ToolResult::denied(format!( + "File too large: {} bytes (limit: {MAX_FILE_SIZE_BYTES} bytes)", + meta.len() + ))); + } + } + Err(e) => { + return Ok(ToolResult::denied(format!( + "Failed to read file metadata: {e}" + ))); + } + } + + match tokio::fs::read_to_string(&resolved_path).await { + Ok(contents) => { + let lines: Vec<&str> = contents.lines().collect(); + let total = lines.len(); + + if total == 0 { + return Ok(ToolResult::ok("")); + } + + let offset = args + .get("offset") + .and_then(|v| v.as_u64()) + .map(|v| { + usize::try_from(v.max(1)) + .unwrap_or(usize::MAX) + .saturating_sub(1) + }) + .unwrap_or(0); + let start = offset.min(total); + + let end = match args.get("limit").and_then(|v| v.as_u64()) { + Some(l) => { + let limit = usize::try_from(l).unwrap_or(usize::MAX); + start.saturating_add(limit).min(total) + } + None => total, + }; + + if start >= end { + return Ok(ToolResult::ok(format!( + "[No lines in range, file has {total} lines]" + ))); + } + + let numbered: String = lines[start..end] + .iter() + .enumerate() + .map(|(i, line)| format!("{}: {}", start + i + 1, line)) + .collect::>() + .join("\n"); + + let partial = start > 0 || end < total; + let summary = if partial { + format!("\n[Lines {}-{} of {total}]", start + 1, end) + } else { + format!("\n[{total} lines total]") + }; + + Ok(ToolResult::ok(format!("{numbered}{summary}"))) + } + Err(_) => { + let bytes = tokio::fs::read(&resolved_path) + .await + .map_err(|e| anyhow::anyhow!("Failed to read file: {e}"))?; + + let lossy = String::from_utf8_lossy(&bytes).into_owned(); + Ok(ToolResult::ok(lossy)) + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::agent::Tool; + use crate::security::AutonomyLevel; + use serde_json::json; + + fn test_security(workspace: std::path::PathBuf) -> Arc { + Arc::new(SecurityPolicy { + workspace_dir: workspace, + ..SecurityPolicy::default() + }) + } + + #[test] + fn file_read_tool_is_not_mutating() { + let security = Arc::new(SecurityPolicy::default()); + let tool = FileReadTool::new(security); + assert!(!tool.is_mutating()); + } + + #[tokio::test] + async fn file_read_existing_file() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("test.txt"), "hello world") + .await + .unwrap(); + + let tool = FileReadTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"path": "test.txt"})).await.unwrap(); + assert!(result.success); + assert!(result.output.contains("1: hello world")); + assert!(result.output.contains("[1 lines total]")); + } + + #[tokio::test] + async fn file_read_nonexistent_file() { + let dir = tempfile::tempdir().unwrap(); + let tool = FileReadTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"path": "no_such_file.txt"})) + .await + .unwrap(); + assert!(!result.success); + assert!(result.error.is_some()); + } + + #[tokio::test] + async fn file_read_blocks_path_traversal() { + let dir = tempfile::tempdir().unwrap(); + let tool = FileReadTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"path": "../../../etc/passwd"})) + .await + .unwrap(); + assert!(!result.success); + assert!(result.error.unwrap().contains("not allowed")); + } + + #[tokio::test] + async fn file_read_blocks_absolute_path() { + let dir = tempfile::tempdir().unwrap(); + let tool = FileReadTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"path": "/etc/passwd"})).await.unwrap(); + assert!(!result.success); + } + + #[tokio::test] + async fn file_read_blocks_when_rate_limited() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("test.txt"), "ok") + .await + .unwrap(); + + let security = Arc::new(SecurityPolicy { + workspace_dir: dir.path().to_path_buf(), + max_actions_per_hour: 0, + ..SecurityPolicy::default() + }); + let tool = FileReadTool::new(security); + let result = tool.execute(json!({"path": "test.txt"})).await.unwrap(); + assert!(!result.success); + assert!(result.error.unwrap().contains("Rate limit")); + } + + #[tokio::test] + async fn file_read_with_offset_and_limit() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write( + dir.path().join("multi.txt"), + "line1\nline2\nline3\nline4\nline5", + ) + .await + .unwrap(); + + let tool = FileReadTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"path": "multi.txt", "offset": 2, "limit": 2})) + .await + .unwrap(); + assert!(result.success); + assert!(result.output.contains("2: line2")); + assert!(result.output.contains("3: line3")); + assert!(!result.output.contains("4: line4")); + } + + #[tokio::test] + async fn file_read_offset_beyond_end() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("short.txt"), "one\ntwo") + .await + .unwrap(); + + let tool = FileReadTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"path": "short.txt", "offset": 100})) + .await + .unwrap(); + assert!(result.success); + assert!(result.output.contains("No lines in range")); + } + + #[tokio::test] + async fn file_read_empty_file() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("empty.txt"), "") + .await + .unwrap(); + + let tool = FileReadTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"path": "empty.txt"})).await.unwrap(); + assert!(result.success); + assert!(result.output.is_empty()); + } + + #[tokio::test] + async fn file_read_nested_path() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::create_dir_all(dir.path().join("sub/dir")) + .await + .unwrap(); + tokio::fs::write(dir.path().join("sub/dir/file.txt"), "nested content") + .await + .unwrap(); + + let tool = FileReadTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"path": "sub/dir/file.txt"})) + .await + .unwrap(); + assert!(result.success); + assert!(result.output.contains("nested content")); + } + + #[cfg(unix)] + #[tokio::test] + async fn file_read_blocks_symlink_escape() { + let workspace = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + tokio::fs::write(outside.path().join("secret.txt"), "secret data") + .await + .unwrap(); + std::os::unix::fs::symlink( + outside.path().join("secret.txt"), + workspace.path().join("link.txt"), + ) + .unwrap(); + + let tool = FileReadTool::new(test_security(workspace.path().to_path_buf())); + let result = tool.execute(json!({"path": "link.txt"})).await.unwrap(); + assert!(!result.success); + } + + #[tokio::test] + async fn file_read_blocks_null_byte_in_path() { + let dir = tempfile::tempdir().unwrap(); + let tool = FileReadTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"path": "file\0.txt"})).await.unwrap(); + assert!(!result.success); + } + + #[tokio::test] + async fn file_read_rejects_oversized_file() { + let dir = tempfile::tempdir().unwrap(); + let big_file = dir.path().join("big.bin"); + // Create a file just over the limit using sparse writing + let f = std::fs::File::create(&big_file).unwrap(); + f.set_len(MAX_FILE_SIZE_BYTES + 1).unwrap(); + + let tool = FileReadTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"path": "big.bin"})).await.unwrap(); + assert!(!result.success); + assert!(result.error.unwrap().contains("too large")); + } + + #[tokio::test] + async fn file_read_lossy_reads_binary_file() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("binary.bin"), b"\xff\xfe\x00\x01hello") + .await + .unwrap(); + + let tool = FileReadTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"path": "binary.bin"})).await.unwrap(); + assert!(result.success); + assert!(result.output.contains("hello")); + } + + #[tokio::test] + async fn file_read_blocks_readonly_not_applicable() { + // ReadOnly mode should still allow file reads (they don't use enforce_tool_operation) + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("file.txt"), "content") + .await + .unwrap(); + + let security = Arc::new(SecurityPolicy { + autonomy: AutonomyLevel::ReadOnly, + workspace_dir: dir.path().to_path_buf(), + ..SecurityPolicy::default() + }); + // FileReadTool uses is_rate_limited + is_path_allowed + record_action + // ReadOnly doesn't block reads via is_path_allowed + let tool = FileReadTool::new(security); + let result = tool.execute(json!({"path": "file.txt"})).await.unwrap(); + assert!(result.success); + } +} diff --git a/crewforge-rs/src/tools/file_write.rs b/crewforge-rs/src/tools/file_write.rs new file mode 100644 index 0000000..59bc9a0 --- /dev/null +++ b/crewforge-rs/src/tools/file_write.rs @@ -0,0 +1,315 @@ +use crate::agent::ToolResult; +use crate::security::SecurityPolicy; +use async_trait::async_trait; +use std::sync::Arc; + +pub struct FileWriteTool { + security: Arc, +} + +impl FileWriteTool { + pub fn new(security: Arc) -> Self { + Self { security } + } +} + +#[async_trait] +impl crate::agent::Tool for FileWriteTool { + fn name(&self) -> &str { + "file_write" + } + + fn description(&self) -> &str { + "Write content to a file. Creates parent directories if needed. Refuses to write through symlinks." + } + + fn parameters(&self) -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "path": { + "type": "string", + "description": "Relative path to write to" + }, + "content": { + "type": "string", + "description": "Content to write to the file" + } + }, + "required": ["path", "content"] + }) + } + + fn is_mutating(&self) -> bool { + true + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + let path = args + .get("path") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing 'path' parameter"))?; + let content = args + .get("content") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing 'content' parameter"))?; + + if !self.security.can_act() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Security policy: read-only mode, cannot write files".into()), + }); + } + + if self.security.is_rate_limited() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Rate limit exceeded".into()), + }); + } + + if !self.security.is_path_allowed(path) { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Path not allowed by security policy: {path}")), + }); + } + + let full_path = self.security.workspace_dir.join(path); + + // Create parent directories + if let Some(parent) = full_path.parent() { + if let Err(e) = tokio::fs::create_dir_all(parent).await { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Failed to create parent directories: {e}")), + }); + } + + // Canonicalize parent to check resolved path + let resolved_parent = match parent.canonicalize() { + Ok(p) => p, + Err(e) => { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Failed to resolve parent path: {e}")), + }); + } + }; + + if !self.security.is_resolved_path_allowed(&resolved_parent) { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some( + self.security + .resolved_path_violation_message(&resolved_parent), + ), + }); + } + } + + // Refuse to write through symlinks + #[cfg(unix)] + if full_path.exists() { + let meta = std::fs::symlink_metadata(&full_path); + if let Ok(m) = meta + && m.file_type().is_symlink() + { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Refusing to write through symlink".into()), + }); + } + } + + if !self.security.record_action() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Rate limit exceeded: action budget exhausted".into()), + }); + } + + match tokio::fs::write(&full_path, content).await { + Ok(()) => Ok(ToolResult { + success: true, + output: format!("Wrote {} bytes to {path}", content.len()), + error: None, + }), + Err(e) => Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Failed to write file: {e}")), + }), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::agent::Tool; + use crate::security::AutonomyLevel; + use serde_json::json; + + fn test_security(workspace: std::path::PathBuf) -> Arc { + Arc::new(SecurityPolicy { + workspace_dir: workspace, + ..SecurityPolicy::default() + }) + } + + #[tokio::test] + async fn file_write_creates_file() { + let dir = tempfile::tempdir().unwrap(); + let tool = FileWriteTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"path": "new.txt", "content": "hello"})) + .await + .unwrap(); + assert!(result.success); + let content = tokio::fs::read_to_string(dir.path().join("new.txt")) + .await + .unwrap(); + assert_eq!(content, "hello"); + } + + #[tokio::test] + async fn file_write_creates_parent_dirs() { + let dir = tempfile::tempdir().unwrap(); + let tool = FileWriteTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"path": "sub/dir/file.txt", "content": "nested"})) + .await + .unwrap(); + assert!(result.success); + assert!(dir.path().join("sub/dir/file.txt").exists()); + } + + #[tokio::test] + async fn file_write_overwrites_existing() { + let dir = tempfile::tempdir().unwrap(); + tokio::fs::write(dir.path().join("exist.txt"), "old") + .await + .unwrap(); + + let tool = FileWriteTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"path": "exist.txt", "content": "new"})) + .await + .unwrap(); + assert!(result.success); + let content = tokio::fs::read_to_string(dir.path().join("exist.txt")) + .await + .unwrap(); + assert_eq!(content, "new"); + } + + #[tokio::test] + async fn file_write_blocks_path_traversal() { + let dir = tempfile::tempdir().unwrap(); + let tool = FileWriteTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"path": "../escape.txt", "content": "bad"})) + .await + .unwrap(); + assert!(!result.success); + } + + #[tokio::test] + async fn file_write_blocks_absolute_path() { + let dir = tempfile::tempdir().unwrap(); + let tool = FileWriteTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"path": "/tmp/bad.txt", "content": "bad"})) + .await + .unwrap(); + assert!(!result.success); + } + + #[tokio::test] + async fn file_write_blocks_readonly_mode() { + let dir = tempfile::tempdir().unwrap(); + let security = Arc::new(SecurityPolicy { + autonomy: AutonomyLevel::ReadOnly, + workspace_dir: dir.path().to_path_buf(), + ..SecurityPolicy::default() + }); + let tool = FileWriteTool::new(security); + let result = tool + .execute(json!({"path": "file.txt", "content": "no"})) + .await + .unwrap(); + assert!(!result.success); + assert!(result.error.unwrap().contains("read-only")); + } + + #[tokio::test] + async fn file_write_blocks_when_rate_limited() { + let dir = tempfile::tempdir().unwrap(); + let security = Arc::new(SecurityPolicy { + workspace_dir: dir.path().to_path_buf(), + max_actions_per_hour: 0, + ..SecurityPolicy::default() + }); + let tool = FileWriteTool::new(security); + let result = tool + .execute(json!({"path": "file.txt", "content": "no"})) + .await + .unwrap(); + assert!(!result.success); + } + + #[cfg(unix)] + #[tokio::test] + async fn file_write_blocks_symlink_escape() { + let workspace = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + std::os::unix::fs::symlink(outside.path(), workspace.path().join("link_dir")).unwrap(); + + let tool = FileWriteTool::new(test_security(workspace.path().to_path_buf())); + let result = tool + .execute(json!({"path": "link_dir/file.txt", "content": "bad"})) + .await + .unwrap(); + assert!(!result.success); + } + + #[cfg(unix)] + #[tokio::test] + async fn file_write_blocks_symlink_target_file() { + let workspace = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + let target = outside.path().join("target.txt"); + tokio::fs::write(&target, "original").await.unwrap(); + std::os::unix::fs::symlink(&target, workspace.path().join("link.txt")).unwrap(); + + let tool = FileWriteTool::new(test_security(workspace.path().to_path_buf())); + let result = tool + .execute(json!({"path": "link.txt", "content": "overwrite"})) + .await + .unwrap(); + assert!(!result.success); + // Verify original file was not modified + let content = tokio::fs::read_to_string(&target).await.unwrap(); + assert_eq!(content, "original"); + } + + #[tokio::test] + async fn file_write_blocks_null_byte_in_path() { + let dir = tempfile::tempdir().unwrap(); + let tool = FileWriteTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"path": "file\0.txt", "content": "bad"})) + .await + .unwrap(); + assert!(!result.success); + } +} diff --git a/crewforge-rs/src/tools/glob_search.rs b/crewforge-rs/src/tools/glob_search.rs new file mode 100644 index 0000000..94667e7 --- /dev/null +++ b/crewforge-rs/src/tools/glob_search.rs @@ -0,0 +1,353 @@ +use crate::agent::ToolResult; +use crate::security::SecurityPolicy; +use async_trait::async_trait; +use std::sync::Arc; + +const MAX_RESULTS: usize = 1000; + +pub struct GlobSearchTool { + security: Arc, +} + +impl GlobSearchTool { + pub fn new(security: Arc) -> Self { + Self { security } + } +} + +#[async_trait] +impl crate::agent::Tool for GlobSearchTool { + fn name(&self) -> &str { + "glob_search" + } + + fn description(&self) -> &str { + "Search for files matching a glob pattern within the workspace. \ + Returns a sorted list of matching file paths relative to the workspace root." + } + + fn parameters(&self) -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "pattern": { + "type": "string", + "description": "Glob pattern to match files, e.g. '**/*.rs', 'src/**/mod.rs'" + } + }, + "required": ["pattern"] + }) + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + let pattern = args + .get("pattern") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing 'pattern' parameter"))?; + + if self.security.is_rate_limited() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Rate limit exceeded".into()), + }); + } + + // Reject absolute paths + if pattern.starts_with('/') || pattern.starts_with('\\') { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Absolute paths are not allowed. Use a relative glob pattern.".into()), + }); + } + + // Reject path traversal + if pattern.contains("../") || pattern.contains("..\\") || pattern == ".." { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Path traversal ('..') is not allowed in glob patterns.".into()), + }); + } + + if !self.security.record_action() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Rate limit exceeded: action budget exhausted".into()), + }); + } + + let workspace = &self.security.workspace_dir; + let full_pattern = workspace.join(pattern).to_string_lossy().to_string(); + let security = self.security.clone(); + let pattern_owned = pattern.to_string(); + + let (results, truncated) = match tokio::task::spawn_blocking(move || { + let entries = match glob::glob(&full_pattern) { + Ok(paths) => paths, + Err(e) => return Err(format!("Invalid glob pattern: {e}")), + }; + + let workspace_canon = match std::fs::canonicalize(&security.workspace_dir) { + Ok(p) => p, + Err(e) => return Err(format!("Cannot resolve workspace directory: {e}")), + }; + + let mut results = Vec::new(); + let mut truncated = false; + + for entry in entries { + let path = match entry { + Ok(p) => p, + Err(_) => continue, + }; + + let resolved = match std::fs::canonicalize(&path) { + Ok(p) => p, + Err(_) => continue, + }; + + if !security.is_resolved_path_allowed(&resolved) { + continue; + } + + if resolved.is_dir() { + continue; + } + + if let Ok(rel) = resolved.strip_prefix(&workspace_canon) { + results.push(rel.to_string_lossy().to_string()); + } + + if results.len() >= MAX_RESULTS { + truncated = true; + break; + } + } + + results.sort(); + Ok((results, truncated)) + }) + .await + .unwrap_or_else(|e| Err(format!("Glob task panicked: {e}"))) + { + Ok(pair) => pair, + Err(msg) => { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(msg), + }); + } + }; + + let pattern = &pattern_owned; + + let output = if results.is_empty() { + format!("No files matching pattern '{pattern}' found in workspace.") + } else { + use std::fmt::Write; + let mut buf = results.join("\n"); + if truncated { + let _ = write!( + buf, + "\n\n[Results truncated: showing first {MAX_RESULTS} of more matches]" + ); + } + let _ = write!(buf, "\n\nTotal: {} files", results.len()); + buf + }; + + Ok(ToolResult { + success: true, + output, + error: None, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::agent::Tool; + use serde_json::json; + + fn test_security(workspace: std::path::PathBuf) -> Arc { + Arc::new(SecurityPolicy { + workspace_dir: workspace, + ..SecurityPolicy::default() + }) + } + + #[tokio::test] + async fn glob_search_single_file() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("hello.txt"), "content").unwrap(); + + let tool = GlobSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"pattern": "hello.txt"})).await.unwrap(); + + assert!(result.success); + assert!(result.output.contains("hello.txt")); + } + + #[tokio::test] + async fn glob_search_multiple_files() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("a.txt"), "").unwrap(); + std::fs::write(dir.path().join("b.txt"), "").unwrap(); + std::fs::write(dir.path().join("c.rs"), "").unwrap(); + + let tool = GlobSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"pattern": "*.txt"})).await.unwrap(); + + assert!(result.success); + assert!(result.output.contains("a.txt")); + assert!(result.output.contains("b.txt")); + assert!(!result.output.contains("c.rs")); + } + + #[tokio::test] + async fn glob_search_recursive() { + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(dir.path().join("sub/deep")).unwrap(); + std::fs::write(dir.path().join("root.txt"), "").unwrap(); + std::fs::write(dir.path().join("sub/mid.txt"), "").unwrap(); + std::fs::write(dir.path().join("sub/deep/leaf.txt"), "").unwrap(); + + let tool = GlobSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"pattern": "**/*.txt"})).await.unwrap(); + + assert!(result.success); + assert!(result.output.contains("root.txt")); + assert!(result.output.contains("mid.txt")); + assert!(result.output.contains("leaf.txt")); + } + + #[tokio::test] + async fn glob_search_no_matches() { + let dir = tempfile::tempdir().unwrap(); + + let tool = GlobSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool + .execute(json!({"pattern": "*.nonexistent"})) + .await + .unwrap(); + + assert!(result.success); + assert!(result.output.contains("No files matching pattern")); + } + + #[tokio::test] + async fn glob_search_rejects_absolute_path() { + let tool = GlobSearchTool::new(test_security(std::env::temp_dir())); + let result = tool.execute(json!({"pattern": "/etc/**/*"})).await.unwrap(); + + assert!(!result.success); + assert!(result.error.as_ref().unwrap().contains("Absolute paths")); + } + + #[tokio::test] + async fn glob_search_rejects_path_traversal() { + let tool = GlobSearchTool::new(test_security(std::env::temp_dir())); + let result = tool + .execute(json!({"pattern": "../../../etc/passwd"})) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.as_ref().unwrap().contains("Path traversal")); + } + + #[cfg(unix)] + #[tokio::test] + async fn glob_search_filters_symlink_escape() { + let root = tempfile::tempdir().unwrap(); + let workspace = root.path().join("workspace"); + let outside = root.path().join("outside"); + + std::fs::create_dir_all(&workspace).unwrap(); + std::fs::create_dir_all(&outside).unwrap(); + std::fs::write(outside.join("secret.txt"), "leaked").unwrap(); + + std::os::unix::fs::symlink(outside.join("secret.txt"), workspace.join("escape.txt")) + .unwrap(); + std::fs::write(workspace.join("legit.txt"), "ok").unwrap(); + + let tool = GlobSearchTool::new(test_security(workspace.clone())); + let result = tool.execute(json!({"pattern": "*.txt"})).await.unwrap(); + + assert!(result.success); + assert!(result.output.contains("legit.txt")); + assert!(!result.output.contains("escape.txt")); + assert!(!result.output.contains("secret.txt")); + } + + #[tokio::test] + async fn glob_search_results_sorted() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("c.txt"), "").unwrap(); + std::fs::write(dir.path().join("a.txt"), "").unwrap(); + std::fs::write(dir.path().join("b.txt"), "").unwrap(); + + let tool = GlobSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"pattern": "*.txt"})).await.unwrap(); + + assert!(result.success); + let lines: Vec<&str> = result.output.lines().collect(); + assert!(lines.len() >= 3); + assert_eq!(lines[0], "a.txt"); + assert_eq!(lines[1], "b.txt"); + assert_eq!(lines[2], "c.txt"); + } + + #[tokio::test] + async fn glob_search_excludes_directories() { + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir(dir.path().join("subdir")).unwrap(); + std::fs::write(dir.path().join("file.txt"), "").unwrap(); + + let tool = GlobSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"pattern": "*"})).await.unwrap(); + + assert!(result.success); + assert!(result.output.contains("file.txt")); + assert!(!result.output.contains("subdir")); + } + + #[tokio::test] + async fn glob_search_rate_limited() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("file.txt"), "").unwrap(); + + let security = Arc::new(SecurityPolicy { + workspace_dir: dir.path().to_path_buf(), + max_actions_per_hour: 0, + ..SecurityPolicy::default() + }); + let tool = GlobSearchTool::new(security); + let result = tool.execute(json!({"pattern": "*.txt"})).await.unwrap(); + + assert!(!result.success); + assert!(result.error.as_ref().unwrap().contains("Rate limit")); + } + + #[tokio::test] + async fn glob_search_invalid_pattern() { + let dir = tempfile::tempdir().unwrap(); + + let tool = GlobSearchTool::new(test_security(dir.path().to_path_buf())); + let result = tool.execute(json!({"pattern": "[invalid"})).await.unwrap(); + + assert!(!result.success); + assert!( + result + .error + .as_ref() + .unwrap() + .contains("Invalid glob pattern") + ); + } +} diff --git a/crewforge-rs/src/tools/mod.rs b/crewforge-rs/src/tools/mod.rs new file mode 100644 index 0000000..87cc976 --- /dev/null +++ b/crewforge-rs/src/tools/mod.rs @@ -0,0 +1,43 @@ +pub mod content_search; +pub mod file_edit; +pub mod file_read; +pub mod file_write; +pub mod glob_search; +pub mod shell; +pub mod traits; + +pub use content_search::ContentSearchTool; +pub use file_edit::FileEditTool; +pub use file_read::FileReadTool; +pub use file_write::FileWriteTool; +pub use glob_search::GlobSearchTool; +pub use shell::ShellTool; +pub use traits::{RuntimeAdapter, TokioRuntime}; + +use crate::agent::Tool; +use crate::security::SecurityPolicy; +use std::sync::Arc; + +/// All built-in tools backed by SecurityPolicy. +pub fn default_tools( + security: Arc, + runtime: Arc, +) -> Vec> { + vec![ + Box::new(ShellTool::new(security.clone(), runtime)), + Box::new(FileReadTool::new(security.clone())), + Box::new(FileWriteTool::new(security.clone())), + Box::new(FileEditTool::new(security.clone())), + Box::new(GlobSearchTool::new(security.clone())), + Box::new(ContentSearchTool::new(security)), + ] +} + +/// All tools including future memory/browser tools. +/// For now, same as default_tools. +pub fn all_tools( + security: Arc, + runtime: Arc, +) -> Vec> { + default_tools(security, runtime) +} diff --git a/crewforge-rs/src/tools/shell.rs b/crewforge-rs/src/tools/shell.rs new file mode 100644 index 0000000..4c7017f --- /dev/null +++ b/crewforge-rs/src/tools/shell.rs @@ -0,0 +1,431 @@ +use crate::agent::ToolResult; +use crate::security::SecurityPolicy; +use crate::security::policy::is_valid_env_var_name; +use crate::tools::traits::RuntimeAdapter; +use async_trait::async_trait; +use std::collections::HashSet; +use std::sync::Arc; +use std::time::Duration; + +const SHELL_TIMEOUT_SECS: u64 = 60; +const MAX_OUTPUT_BYTES: usize = 1_048_576; // 1 MB +const SAFE_ENV_VARS: &[&str] = &[ + "PATH", "HOME", "TERM", "LANG", "LC_ALL", "LC_CTYPE", "USER", "SHELL", "TMPDIR", +]; + +pub struct ShellTool { + security: Arc, + runtime: Arc, +} + +impl ShellTool { + pub fn new(security: Arc, runtime: Arc) -> Self { + Self { security, runtime } + } +} + +pub(crate) fn collect_allowed_shell_env_vars(security: &SecurityPolicy) -> Vec { + let mut out = Vec::new(); + let mut seen = HashSet::new(); + for key in SAFE_ENV_VARS + .iter() + .copied() + .chain(security.shell_env_passthrough.iter().map(|s| s.as_str())) + { + let candidate = key.trim(); + if candidate.is_empty() || !is_valid_env_var_name(candidate) { + continue; + } + if seen.insert(candidate.to_string()) { + out.push(candidate.to_string()); + } + } + out +} + +fn extract_command_argument(args: &serde_json::Value) -> Option { + if let Some(command) = args + .get("command") + .and_then(|v| v.as_str()) + .map(str::trim) + .filter(|cmd| !cmd.is_empty()) + { + return Some(command.to_string()); + } + + for alias in [ + "cmd", + "script", + "shell_command", + "command_line", + "bash", + "sh", + "input", + ] { + if let Some(command) = args + .get(alias) + .and_then(|v| v.as_str()) + .map(str::trim) + .filter(|cmd| !cmd.is_empty()) + { + return Some(command.to_string()); + } + } + + args.as_str() + .map(str::trim) + .filter(|cmd| !cmd.is_empty()) + .map(ToString::to_string) +} + +fn truncate_utf8(s: &str, max_bytes: usize) -> usize { + if s.len() <= max_bytes { + return s.len(); + } + let mut end = max_bytes; + while end > 0 && !s.is_char_boundary(end) { + end -= 1; + } + end +} + +#[async_trait] +impl crate::agent::Tool for ShellTool { + fn name(&self) -> &str { + "shell" + } + + fn description(&self) -> &str { + "Execute a shell command in the workspace directory" + } + + fn parameters(&self) -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "command": { + "type": "string", + "description": "The shell command to execute" + }, + "approved": { + "type": "boolean", + "description": "Set true to explicitly approve medium/high-risk commands", + "default": false + } + }, + "required": ["command"] + }) + } + + fn is_mutating(&self) -> bool { + true + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + let command = extract_command_argument(&args) + .ok_or_else(|| anyhow::anyhow!("Missing 'command' parameter"))?; + let approved = args + .get("approved") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + + if self.security.is_rate_limited() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Rate limit exceeded".into()), + }); + } + + match self.security.validate_command_execution(&command, approved) { + Ok(_) => {} + Err(reason) => { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(reason), + }); + } + } + + if let Some(path) = self.security.forbidden_path_argument(&command) { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Path blocked by security policy: {path}")), + }); + } + + if !self.security.record_action() { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("Rate limit exceeded: action budget exhausted".into()), + }); + } + + let mut cmd = match self + .runtime + .build_shell_command(&command, &self.security.workspace_dir) + { + Ok(cmd) => cmd, + Err(e) => { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Failed to build runtime command: {e}")), + }); + } + }; + // Environment sandboxing: clear all env vars then selectively restore + // safe ones. This is intentionally done here (not in RuntimeAdapter) so + // the security policy controls which vars are passed through. + cmd.env_clear(); + + for var in collect_allowed_shell_env_vars(&self.security) { + if let Ok(val) = std::env::var(&var) { + cmd.env(&var, val); + } + } + + let result = + tokio::time::timeout(Duration::from_secs(SHELL_TIMEOUT_SECS), cmd.output()).await; + + match result { + Ok(Ok(output)) => { + let mut stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let mut stderr = String::from_utf8_lossy(&output.stderr).to_string(); + + if stdout.len() > MAX_OUTPUT_BYTES { + let boundary = truncate_utf8(&stdout, MAX_OUTPUT_BYTES); + stdout.truncate(boundary); + stdout.push_str("\n... [output truncated at 1MB]"); + } + if stderr.len() > MAX_OUTPUT_BYTES { + let boundary = truncate_utf8(&stderr, MAX_OUTPUT_BYTES); + stderr.truncate(boundary); + stderr.push_str("\n... [stderr truncated at 1MB]"); + } + + Ok(ToolResult { + success: output.status.success(), + output: stdout, + error: if stderr.is_empty() { + None + } else { + Some(stderr) + }, + }) + } + Ok(Err(e)) => Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!("Failed to execute command: {e}")), + }), + Err(_) => Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!( + "Command timed out after {SHELL_TIMEOUT_SECS}s and was killed" + )), + }), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::agent::Tool; + use crate::security::AutonomyLevel; + use crate::tools::traits::TokioRuntime; + use serde_json::json; + + fn test_security(autonomy: AutonomyLevel) -> Arc { + Arc::new(SecurityPolicy { + autonomy, + workspace_dir: std::env::temp_dir(), + ..SecurityPolicy::default() + }) + } + + fn test_runtime() -> Arc { + Arc::new(TokioRuntime) + } + + #[test] + fn shell_tool_is_mutating() { + let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime()); + assert!(tool.is_mutating()); + } + + #[test] + fn shell_name_and_schema() { + let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime()); + assert_eq!(tool.name(), "shell"); + let schema = tool.parameters(); + assert!(schema["properties"]["command"].is_object()); + assert!( + schema["required"] + .as_array() + .unwrap() + .contains(&json!("command")) + ); + } + + #[tokio::test] + async fn shell_executes_simple_command() { + let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime()); + let result = tool + .execute(json!({"command": "echo hello"})) + .await + .unwrap(); + assert!(result.success); + assert!(result.output.trim().contains("hello")); + } + + #[tokio::test] + async fn shell_blocks_dangerous_command() { + let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime()); + let result = tool.execute(json!({"command": "rm -rf /"})).await.unwrap(); + assert!(!result.success); + } + + #[tokio::test] + async fn shell_blocks_rate_limited() { + let security = Arc::new(SecurityPolicy { + autonomy: AutonomyLevel::Supervised, + max_actions_per_hour: 0, + workspace_dir: std::env::temp_dir(), + ..SecurityPolicy::default() + }); + let tool = ShellTool::new(security, test_runtime()); + let result = tool.execute(json!({"command": "echo test"})).await.unwrap(); + assert!(!result.success); + assert!(result.error.as_deref().unwrap_or("").contains("Rate limit")); + } + + #[tokio::test] + async fn shell_blocks_readonly_mode() { + let tool = ShellTool::new(test_security(AutonomyLevel::ReadOnly), test_runtime()); + let result = tool.execute(json!({"command": "ls"})).await.unwrap(); + assert!(!result.success); + assert!(result.error.as_ref().unwrap().contains("not allowed")); + } + + #[tokio::test] + async fn shell_forbidden_path_in_args() { + let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime()); + let result = tool + .execute(json!({"command": "cat /etc/passwd"})) + .await + .unwrap(); + assert!(!result.success); + assert!( + result + .error + .as_deref() + .unwrap_or("") + .contains("Path blocked") + ); + } + + #[tokio::test] + async fn shell_captures_exit_code() { + let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime()); + let result = tool + .execute(json!({"command": "ls /nonexistent_dir_xyz"})) + .await + .unwrap(); + assert!(!result.success); + } + + #[tokio::test] + async fn shell_captures_stderr() { + let tool = ShellTool::new(test_security(AutonomyLevel::Full), test_runtime()); + let result = tool + .execute(json!({"command": "echo error_msg >&2"})) + .await + .unwrap(); + assert!(result.error.as_deref().unwrap_or("").contains("error_msg")); + } + + #[tokio::test] + async fn shell_missing_command_param() { + let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime()); + let result = tool.execute(json!({})).await; + assert!(result.is_err()); + } + + #[test] + fn extract_command_supports_aliases() { + assert_eq!( + extract_command_argument(&json!({"cmd": "echo from-cmd"})).as_deref(), + Some("echo from-cmd") + ); + assert_eq!( + extract_command_argument(&json!({"script": "echo from-script"})).as_deref(), + Some("echo from-script") + ); + assert_eq!( + extract_command_argument(&json!("echo from-string")).as_deref(), + Some("echo from-string") + ); + } + + #[test] + fn shell_safe_env_vars_excludes_secrets() { + for var in SAFE_ENV_VARS { + let lower = var.to_lowercase(); + assert!( + !lower.contains("key") && !lower.contains("secret") && !lower.contains("token"), + "SAFE_ENV_VARS must not include: {var}" + ); + } + } + + #[test] + fn invalid_env_var_names_are_filtered() { + let security = SecurityPolicy { + shell_env_passthrough: vec![ + "VALID_NAME".into(), + "BAD-NAME".into(), + "1NOPE".into(), + "ALSO_VALID".into(), + ], + ..SecurityPolicy::default() + }; + let vars = collect_allowed_shell_env_vars(&security); + assert!(vars.contains(&"VALID_NAME".to_string())); + assert!(vars.contains(&"ALSO_VALID".to_string())); + assert!(!vars.contains(&"BAD-NAME".to_string())); + assert!(!vars.contains(&"1NOPE".to_string())); + } + + #[tokio::test] + async fn shell_record_action_budget_exhaustion() { + let security = Arc::new(SecurityPolicy { + autonomy: AutonomyLevel::Full, + max_actions_per_hour: 1, + workspace_dir: std::env::temp_dir(), + ..SecurityPolicy::default() + }); + let tool = ShellTool::new(security, test_runtime()); + + let r1 = tool + .execute(json!({"command": "echo first"})) + .await + .unwrap(); + assert!(r1.success); + + let r2 = tool + .execute(json!({"command": "echo second"})) + .await + .unwrap(); + assert!(!r2.success); + assert!( + r2.error.as_deref().unwrap_or("").contains("Rate limit") + || r2.error.as_deref().unwrap_or("").contains("budget") + ); + } +} diff --git a/crewforge-rs/src/tools/traits.rs b/crewforge-rs/src/tools/traits.rs new file mode 100644 index 0000000..2e63ed6 --- /dev/null +++ b/crewforge-rs/src/tools/traits.rs @@ -0,0 +1,55 @@ +use std::path::Path; + +/// Abstracts shell command execution for testability. +pub trait RuntimeAdapter: Send + Sync { + fn build_shell_command( + &self, + command: &str, + workspace_dir: &Path, + ) -> anyhow::Result; +} + +/// Native runtime: executes shell commands via `sh -c` on Unix. +pub struct TokioRuntime; + +impl RuntimeAdapter for TokioRuntime { + fn build_shell_command( + &self, + command: &str, + workspace_dir: &Path, + ) -> anyhow::Result { + let mut cmd = tokio::process::Command::new("sh"); + cmd.arg("-c").arg(command); + cmd.current_dir(workspace_dir); + cmd.stdout(std::process::Stdio::piped()); + cmd.stderr(std::process::Stdio::piped()); + Ok(cmd) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn tokio_runtime_echo() { + let rt = TokioRuntime; + let dir = std::env::current_dir().unwrap(); + let mut cmd = rt.build_shell_command("echo hello", &dir).unwrap(); + let output = cmd.output().await.unwrap(); + assert!(output.status.success()); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.trim() == "hello"); + } + + #[tokio::test] + async fn tokio_runtime_sets_cwd() { + let rt = TokioRuntime; + let dir = tempfile::tempdir().unwrap(); + let mut cmd = rt.build_shell_command("pwd", dir.path()).unwrap(); + let output = cmd.output().await.unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout); + let canonical = dir.path().canonicalize().unwrap(); + assert_eq!(stdout.trim(), canonical.to_str().unwrap()); + } +} diff --git a/crewforge-rs/src/tui.rs b/crewforge-rs/src/tui.rs index 75582bb..ddb3025 100644 --- a/crewforge-rs/src/tui.rs +++ b/crewforge-rs/src/tui.rs @@ -338,7 +338,9 @@ fn prefixed_message_lines( if density.is_comfort() { spans.push(Span::styled( " ", - Style::default().fg(Color::DarkGray).add_modifier(Modifier::DIM), + Style::default() + .fg(Color::DarkGray) + .add_modifier(Modifier::DIM), )); } spans.push(Span::raw(line)); @@ -415,7 +417,10 @@ fn agent_status_symbol(state: AgentStatusState) -> &'static str { } } -fn build_status_line(statuses: &BTreeMap, density: UiDensity) -> Line<'static> { +fn build_status_line( + statuses: &BTreeMap, + density: UiDensity, +) -> Line<'static> { let has_running = statuses .values() .any(|entry| matches!(entry.state, AgentStatusState::Active)); @@ -430,7 +435,9 @@ fn build_status_line(statuses: &BTreeMap, density: UiD let density_style = if density.is_comfort() { Style::default().fg(Color::Cyan).add_modifier(Modifier::DIM) } else { - Style::default().fg(Color::Yellow).add_modifier(Modifier::DIM) + Style::default() + .fg(Color::Yellow) + .add_modifier(Modifier::DIM) }; let mut spans = vec![ @@ -596,7 +603,11 @@ impl RenderedLineCache { fn ensure_rows(&mut self, lines: &[DisplayLine], view_width: u16, density: UiDensity) { let view_width = view_width.max(1); - if !self.valid || self.width != view_width || self.len != lines.len() || self.density != density { + if !self.valid + || self.width != view_width + || self.len != lines.len() + || self.density != density + { self.rows = rendered_rows_for_lines(lines, view_width, density); self.width = view_width; self.len = lines.len(); @@ -1074,8 +1085,8 @@ pub async fn run_tui_loop( }, if pending_prefetch.is_some() => { let (start, join) = prefetch_join; let events = join.context("failed joining history prefetch task")??; - if history_pager.next_older_start() == Some(start) { - if apply_loaded_history_events( + if history_pager.next_older_start() == Some(start) + && apply_loaded_history_events( start, events, &mut history_pager, @@ -1088,7 +1099,6 @@ pub async fn run_tui_loop( ) { mark_render_dirty(&mut render_dirty, &mut render_pending_since); } - } } maybe_line = msg_rx.recv() => { match maybe_line { @@ -1359,6 +1369,7 @@ fn maybe_start_history_prefetch( *pending_prefetch = Some(HistoryPrefetch { start, task }); } +#[allow(clippy::too_many_arguments)] async fn prepend_next_older_page( history_pager: &mut SessionHistoryPager, runtime: &ChatRuntime, @@ -1407,6 +1418,7 @@ async fn prepend_next_older_page( .await } +#[allow(clippy::too_many_arguments)] fn apply_loaded_history_events( start: usize, events: Vec, @@ -1444,7 +1456,8 @@ fn prepend_history_lines( if older_lines.is_empty() { return false; } - let added_rows = rendered_line_count(&older_lines, view_width, density).min(u16::MAX as usize) as u16; + let added_rows = + rendered_line_count(&older_lines, view_width, density).min(u16::MAX as usize) as u16; display_lines.splice(0..0, older_lines); rendered_line_cache.invalidate(); *scroll_offset = scroll_offset.saturating_add(added_rows); @@ -1602,7 +1615,11 @@ fn rendered_line_count(lines: &[DisplayLine], view_width: u16, density: UiDensit rendered_rows_for_lines(lines, view_width, density).len() } -fn rendered_rows_for_lines(lines: &[DisplayLine], view_width: u16, density: UiDensity) -> Vec> { +fn rendered_rows_for_lines( + lines: &[DisplayLine], + view_width: u16, + density: UiDensity, +) -> Vec> { let view_width = view_width.max(1) as usize; let mut rows = Vec::new(); for line in lines {