diff --git a/Cargo.lock b/Cargo.lock index de5594c..f2363d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1204,6 +1204,7 @@ dependencies = [ name = "cortex-exec" version = "0.0.6" dependencies = [ + "cortex-common", "cortex-engine", "cortex-protocol", "serde_json", diff --git a/src/cortex-common/src/lib.rs b/src/cortex-common/src/lib.rs index 0f30c44..4cfc023 100644 --- a/src/cortex-common/src/lib.rs +++ b/src/cortex-common/src/lib.rs @@ -18,6 +18,7 @@ pub mod signal_safety; pub mod subprocess_env; pub mod subprocess_output; pub mod text_sanitize; +pub mod timeout; pub mod truncate; #[cfg(feature = "cli")] @@ -73,6 +74,11 @@ pub use subprocess_output::{ pub use text_sanitize::{ has_control_chars, normalize_code_fences, sanitize_control_chars, sanitize_for_terminal, }; +pub use timeout::{ + DEFAULT_BATCH_TIMEOUT_SECS, DEFAULT_EXEC_TIMEOUT_SECS, DEFAULT_HEALTH_CHECK_TIMEOUT_SECS, + DEFAULT_READ_TIMEOUT_SECS, DEFAULT_REQUEST_TIMEOUT_SECS, DEFAULT_SHUTDOWN_TIMEOUT_SECS, + DEFAULT_STREAMING_TIMEOUT_SECS, +}; pub use truncate::{ truncate_command, truncate_first_line, truncate_for_display, truncate_id, truncate_id_default, truncate_model_name, truncate_with_ellipsis, truncate_with_unicode_ellipsis, diff --git a/src/cortex-common/src/timeout.rs b/src/cortex-common/src/timeout.rs new file mode 100644 index 0000000..1f7ec37 --- /dev/null +++ b/src/cortex-common/src/timeout.rs @@ -0,0 +1,65 @@ +//! Centralized timeout constants for the Cortex CLI. +//! +//! This module provides consistent timeout values used throughout the codebase. +//! Centralizing these values ensures uniformity and makes it easier to adjust +//! timeouts across the application. + +/// Default timeout for the entire execution in seconds (10 minutes). +/// +/// This is the maximum time allowed for a complete headless execution, +/// including all LLM requests and tool executions. +pub const DEFAULT_EXEC_TIMEOUT_SECS: u64 = 600; + +/// Default timeout for a single LLM request in seconds (2 minutes). +/// +/// This is the maximum time to wait for a single completion request +/// to the LLM provider. +pub const DEFAULT_REQUEST_TIMEOUT_SECS: u64 = 120; + +/// Default timeout for streaming responses in seconds (5 minutes). +/// +/// Extended timeout for LLM streaming requests where responses are +/// delivered incrementally over time. +pub const DEFAULT_STREAMING_TIMEOUT_SECS: u64 = 300; + +/// Default timeout for health check requests in seconds (5 seconds). +/// +/// Short timeout used for quick health check endpoints. +pub const DEFAULT_HEALTH_CHECK_TIMEOUT_SECS: u64 = 5; + +/// Default timeout for graceful shutdown in seconds (30 seconds). +/// +/// Maximum time to wait for in-flight operations to complete during +/// shutdown before forcing termination. +pub const DEFAULT_SHUTDOWN_TIMEOUT_SECS: u64 = 30; + +/// Default timeout for batch execution in seconds (5 minutes). +/// +/// Maximum time allowed for executing a batch of parallel tool calls. +pub const DEFAULT_BATCH_TIMEOUT_SECS: u64 = 300; + +/// Default timeout for individual read operations in seconds (30 seconds). +/// +/// Timeout for individual read operations to prevent hangs when +/// Content-Length doesn't match actual body size. +pub const DEFAULT_READ_TIMEOUT_SECS: u64 = 30; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_timeout_values_are_reasonable() { + // Exec timeout should be greater than request timeout + assert!(DEFAULT_EXEC_TIMEOUT_SECS > DEFAULT_REQUEST_TIMEOUT_SECS); + + // Streaming timeout should be greater than request timeout + assert!(DEFAULT_STREAMING_TIMEOUT_SECS > DEFAULT_REQUEST_TIMEOUT_SECS); + + // Health check should be short + assert!(DEFAULT_HEALTH_CHECK_TIMEOUT_SECS <= 10); + + // Batch timeout should be reasonable + assert!(DEFAULT_BATCH_TIMEOUT_SECS >= 60); + } +} diff --git a/src/cortex-engine/src/tools/handlers/batch.rs b/src/cortex-engine/src/tools/handlers/batch.rs index 81ec81f..e3d4542 100644 --- a/src/cortex-engine/src/tools/handlers/batch.rs +++ b/src/cortex-engine/src/tools/handlers/batch.rs @@ -20,8 +20,9 @@ use crate::tools::spec::{ToolDefinition, ToolHandler, ToolResult}; /// Maximum number of tools that can be executed in a batch. pub const MAX_BATCH_SIZE: usize = 10; -/// Default timeout for batch execution in seconds. -pub const DEFAULT_BATCH_TIMEOUT_SECS: u64 = 300; +/// Default timeout for individual tool execution in seconds. +/// This prevents a single tool from consuming the entire batch timeout. +pub const DEFAULT_TOOL_TIMEOUT_SECS: u64 = 60; /// Tools that cannot be called within a batch (prevent recursion). /// Note: Task is now allowed to enable parallel task execution. @@ -45,6 +46,10 @@ pub struct BatchToolArgs { /// Optional timeout in seconds for the entire batch (default: 300s). #[serde(default)] pub timeout_secs: Option, + /// Optional timeout in seconds for individual tools (default: 60s). + /// This prevents a single tool from consuming the entire batch timeout. + #[serde(default)] + pub tool_timeout_secs: Option, } /// Result of a single tool call within the batch. @@ -158,7 +163,7 @@ impl BatchToolHandler { &self, calls: Vec, context: &ToolContext, - timeout_duration: Duration, + tool_timeout: Duration, ) -> BatchResult { let start_time = Instant::now(); let results = Arc::new(Mutex::new(Vec::with_capacity(calls.len()))); @@ -176,9 +181,9 @@ impl BatchToolHandler { async move { let call_start = Instant::now(); - // Execute with per-call timeout (use batch timeout for each call) + // Execute with per-tool timeout to prevent single tools from blocking others let execution_result = timeout( - timeout_duration, + tool_timeout, executor.execute_tool(&call.tool, call.arguments, &ctx), ) .await; @@ -202,7 +207,7 @@ impl BatchToolHandler { duration_ms, }, Ok(Err(e)) => BatchCallResult { - tool: tool_name, + tool: tool_name.clone(), index, success: false, result: None, @@ -210,11 +215,15 @@ impl BatchToolHandler { duration_ms, }, Err(_) => BatchCallResult { - tool: tool_name, + tool: tool_name.clone(), index, success: false, result: None, - error: Some(format!("Timeout after {}s", timeout_duration.as_secs())), + error: Some(format!( + "Tool '{}' timed out after {}s", + tool_name, + tool_timeout.as_secs() + )), duration_ms, }, }; @@ -328,13 +337,13 @@ impl ToolHandler for BatchToolHandler { // Validate calls self.validate_calls(&args.calls)?; - // Determine timeout - let timeout_secs = args.timeout_secs.unwrap_or(DEFAULT_BATCH_TIMEOUT_SECS); - let timeout_duration = Duration::from_secs(timeout_secs); + // Determine per-tool timeout (prevents single tool from blocking others) + let tool_timeout_secs = args.tool_timeout_secs.unwrap_or(DEFAULT_TOOL_TIMEOUT_SECS); + let tool_timeout = Duration::from_secs(tool_timeout_secs); // Execute all tools in parallel let batch_result = self - .execute_parallel(args.calls, context, timeout_duration) + .execute_parallel(args.calls, context, tool_timeout) .await; // Format output @@ -384,6 +393,12 @@ pub fn batch_tool_definition() -> ToolDefinition { "description": "Optional timeout in seconds for the entire batch execution (default: 300)", "minimum": 1, "maximum": 600 + }, + "tool_timeout_secs": { + "type": "integer", + "description": "Optional timeout in seconds for individual tool execution (default: 60). Prevents a single tool from consuming the entire batch timeout.", + "minimum": 1, + "maximum": 300 } } }), @@ -409,6 +424,7 @@ pub async fn execute_batch( }) .collect(), timeout_secs: None, + tool_timeout_secs: None, }; let arguments = serde_json::to_value(args) diff --git a/src/cortex-engine/src/tools/handlers/mod.rs b/src/cortex-engine/src/tools/handlers/mod.rs index ccfd955..8e7d3a1 100644 --- a/src/cortex-engine/src/tools/handlers/mod.rs +++ b/src/cortex-engine/src/tools/handlers/mod.rs @@ -23,9 +23,10 @@ mod web_search; pub use apply_patch::ApplyPatchHandler; pub use batch::{ BatchCallResult, BatchParams, BatchResult, BatchToolArgs, BatchToolCall, BatchToolExecutor, - BatchToolHandler, DEFAULT_BATCH_TIMEOUT_SECS, DISALLOWED_TOOLS, LegacyBatchToolCall, - MAX_BATCH_SIZE, batch_tool_definition, execute_batch, + BatchToolHandler, DISALLOWED_TOOLS, LegacyBatchToolCall, MAX_BATCH_SIZE, batch_tool_definition, + execute_batch, }; +pub use cortex_common::DEFAULT_BATCH_TIMEOUT_SECS; pub use create_agent::CreateAgentHandler; pub use edit_file::PatchHandler; diff --git a/src/cortex-engine/src/tools/unified_executor.rs b/src/cortex-engine/src/tools/unified_executor.rs index 985fefb..ed083a1 100644 --- a/src/cortex-engine/src/tools/unified_executor.rs +++ b/src/cortex-engine/src/tools/unified_executor.rs @@ -466,6 +466,7 @@ impl UnifiedToolExecutor { Ok(BatchToolArgs { calls, timeout_secs: arguments.get("timeout_secs").and_then(|v| v.as_u64()), + tool_timeout_secs: arguments.get("tool_timeout_secs").and_then(|v| v.as_u64()), }) } else { Err(CortexError::InvalidInput( diff --git a/src/cortex-exec/Cargo.toml b/src/cortex-exec/Cargo.toml index 2693c7c..de3b312 100644 --- a/src/cortex-exec/Cargo.toml +++ b/src/cortex-exec/Cargo.toml @@ -13,6 +13,7 @@ path = "src/lib.rs" workspace = true [dependencies] +cortex-common = { workspace = true } cortex-engine = { workspace = true } cortex-protocol = { workspace = true } tokio = { workspace = true, features = ["full"] } diff --git a/src/cortex-exec/src/runner.rs b/src/cortex-exec/src/runner.rs index e831324..fa39aba 100644 --- a/src/cortex-exec/src/runner.rs +++ b/src/cortex-exec/src/runner.rs @@ -14,6 +14,7 @@ use std::time::Duration; use tokio_stream::StreamExt; +use cortex_common::{DEFAULT_EXEC_TIMEOUT_SECS, DEFAULT_REQUEST_TIMEOUT_SECS}; use cortex_engine::{ Config, ConversationManager, CortexError, client::{ @@ -26,15 +27,25 @@ use cortex_protocol::ConversationId; use crate::output::{OutputFormat, OutputWriter}; -/// Default timeout for the entire execution (10 minutes). -const DEFAULT_TIMEOUT_SECS: u64 = 600; - -/// Default timeout for a single LLM request (2 minutes). -const DEFAULT_REQUEST_TIMEOUT_SECS: u64 = 120; +// ============================================================================ +// CONFIGURATION CONSTANTS +// ============================================================================ /// Maximum retries for transient errors. const MAX_RETRIES: usize = 3; +/// Default maximum number of conversation turns before stopping. +const DEFAULT_MAX_TURNS: usize = 10; + +/// Default temperature for LLM responses (0.0 = deterministic, 1.0 = creative). +const DEFAULT_TEMPERATURE: f32 = 0.7; + +/// Default maximum output tokens for LLM responses. +const DEFAULT_MAX_OUTPUT_TOKENS: u32 = 4096; + +/// Base delay between retry attempts in milliseconds (used for exponential backoff). +const RETRY_DELAY_BASE_MS: u64 = 500; + /// Options for headless execution. #[derive(Debug, Clone)] pub struct ExecOptions { @@ -74,8 +85,8 @@ impl Default for ExecOptions { model: None, output_format: OutputFormat::Text, full_auto: false, - max_turns: Some(10), - timeout_secs: Some(DEFAULT_TIMEOUT_SECS), + max_turns: Some(DEFAULT_MAX_TURNS), + timeout_secs: Some(DEFAULT_EXEC_TIMEOUT_SECS), request_timeout_secs: Some(DEFAULT_REQUEST_TIMEOUT_SECS), sandbox: true, system_prompt: None, @@ -267,7 +278,7 @@ impl ExecRunner { /// Run the execution with full timeout enforcement. pub async fn run(&mut self) -> Result { let timeout = - Duration::from_secs(self.options.timeout_secs.unwrap_or(DEFAULT_TIMEOUT_SECS)); + Duration::from_secs(self.options.timeout_secs.unwrap_or(DEFAULT_EXEC_TIMEOUT_SECS)); // Wrap the entire execution in a timeout match tokio::time::timeout(timeout, self.run_inner()).await { @@ -339,7 +350,7 @@ impl ExecRunner { // Get tool definitions let tools = self.get_tool_definitions(); - let max_turns = self.options.max_turns.unwrap_or(10); + let max_turns = self.options.max_turns.unwrap_or(DEFAULT_MAX_TURNS); // Main execution loop while turns < max_turns { @@ -479,8 +490,8 @@ impl ExecRunner { let request = CompletionRequest { messages: conversation.messages().to_vec(), model: client.model().to_string(), - max_tokens: Some(4096), - temperature: Some(0.7), + max_tokens: Some(DEFAULT_MAX_OUTPUT_TOKENS), + temperature: Some(DEFAULT_TEMPERATURE), seed: None, tools: tools.to_vec(), stream: self.options.streaming, @@ -502,7 +513,7 @@ impl ExecRunner { MAX_RETRIES )); // Exponential backoff - tokio::time::sleep(Duration::from_millis(500 * 2u64.pow(attempt as u32))).await; + tokio::time::sleep(Duration::from_millis(RETRY_DELAY_BASE_MS * 2u64.pow(attempt as u32))).await; } let result = tokio::time::timeout(request_timeout, async { @@ -765,8 +776,8 @@ mod tests { assert!(opts.prompt.is_empty()); assert!(opts.sandbox); - assert_eq!(opts.max_turns, Some(10)); - assert_eq!(opts.timeout_secs, Some(DEFAULT_TIMEOUT_SECS)); + assert_eq!(opts.max_turns, Some(DEFAULT_MAX_TURNS)); + assert_eq!(opts.timeout_secs, Some(DEFAULT_EXEC_TIMEOUT_SECS)); assert!(!opts.full_auto); assert!(opts.streaming); }