From a22a3689a6e82b4b88dcc56cbd3119c8f66c3575 Mon Sep 17 00:00:00 2001 From: echobt Date: Wed, 4 Feb 2026 14:31:13 +0000 Subject: [PATCH 1/3] refactor: centralize timeout constants into cortex-common module --- Cargo.lock | 1 + src/cortex-common/src/lib.rs | 6 ++ src/cortex-common/src/timeout.rs | 65 +++++++++++++++++++ src/cortex-engine/src/tools/handlers/batch.rs | 4 +- src/cortex-engine/src/tools/handlers/mod.rs | 5 +- src/cortex-exec/Cargo.toml | 1 + src/cortex-exec/src/runner.rs | 13 ++-- 7 files changed, 81 insertions(+), 14 deletions(-) create mode 100644 src/cortex-common/src/timeout.rs 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..9450967 100644 --- a/src/cortex-engine/src/tools/handlers/batch.rs +++ b/src/cortex-engine/src/tools/handlers/batch.rs @@ -7,6 +7,7 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use async_trait::async_trait; +use cortex_common::DEFAULT_BATCH_TIMEOUT_SECS; use futures::future::join_all; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; @@ -20,9 +21,6 @@ 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; - /// Tools that cannot be called within a batch (prevent recursion). /// Note: Task is now allowed to enable parallel task execution. pub const DISALLOWED_TOOLS: &[&str] = &["Batch", "batch", "Agent", "agent"]; 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-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..b2c7b93 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,12 +27,6 @@ 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; - /// Maximum retries for transient errors. const MAX_RETRIES: usize = 3; @@ -75,7 +70,7 @@ impl Default for ExecOptions { output_format: OutputFormat::Text, full_auto: false, max_turns: Some(10), - timeout_secs: Some(DEFAULT_TIMEOUT_SECS), + timeout_secs: Some(DEFAULT_EXEC_TIMEOUT_SECS), request_timeout_secs: Some(DEFAULT_REQUEST_TIMEOUT_SECS), sandbox: true, system_prompt: None, @@ -267,7 +262,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 { @@ -766,7 +761,7 @@ 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.timeout_secs, Some(DEFAULT_EXEC_TIMEOUT_SECS)); assert!(!opts.full_auto); assert!(opts.streaming); } From 2878d46ac47782492032ccf93709f4c283ea5099 Mon Sep 17 00:00:00 2001 From: echobt Date: Wed, 4 Feb 2026 14:49:10 +0000 Subject: [PATCH 2/3] feat: add per-tool timeout in batch execution --- src/cortex-engine/src/tools/handlers/batch.rs | 40 ++++++++++++++----- .../src/tools/unified_executor.rs | 1 + 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/cortex-engine/src/tools/handlers/batch.rs b/src/cortex-engine/src/tools/handlers/batch.rs index 9450967..e3d4542 100644 --- a/src/cortex-engine/src/tools/handlers/batch.rs +++ b/src/cortex-engine/src/tools/handlers/batch.rs @@ -7,7 +7,6 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use async_trait::async_trait; -use cortex_common::DEFAULT_BATCH_TIMEOUT_SECS; use futures::future::join_all; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; @@ -21,6 +20,10 @@ 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 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. pub const DISALLOWED_TOOLS: &[&str] = &["Batch", "batch", "Agent", "agent"]; @@ -43,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. @@ -156,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()))); @@ -174,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; @@ -200,7 +207,7 @@ impl BatchToolHandler { duration_ms, }, Ok(Err(e)) => BatchCallResult { - tool: tool_name, + tool: tool_name.clone(), index, success: false, result: None, @@ -208,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, }, }; @@ -326,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 @@ -382,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 } } }), @@ -407,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/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( From d10c1c5f449fbda4aa27203a27c6e888ff99d2c5 Mon Sep 17 00:00:00 2001 From: echobt Date: Wed, 4 Feb 2026 15:02:19 +0000 Subject: [PATCH 3/3] refactor: replace magic numbers with documented named constants --- src/cortex-exec/src/runner.rs | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/cortex-exec/src/runner.rs b/src/cortex-exec/src/runner.rs index b2c7b93..fa39aba 100644 --- a/src/cortex-exec/src/runner.rs +++ b/src/cortex-exec/src/runner.rs @@ -27,9 +27,25 @@ use cortex_protocol::ConversationId; use crate::output::{OutputFormat, OutputWriter}; +// ============================================================================ +// 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 { @@ -69,7 +85,7 @@ impl Default for ExecOptions { model: None, output_format: OutputFormat::Text, full_auto: false, - max_turns: Some(10), + max_turns: Some(DEFAULT_MAX_TURNS), timeout_secs: Some(DEFAULT_EXEC_TIMEOUT_SECS), request_timeout_secs: Some(DEFAULT_REQUEST_TIMEOUT_SECS), sandbox: true, @@ -334,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 { @@ -474,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, @@ -497,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 { @@ -760,7 +776,7 @@ mod tests { assert!(opts.prompt.is_empty()); assert!(opts.sandbox); - assert_eq!(opts.max_turns, Some(10)); + 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);