Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/cortex-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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,
Expand Down
65 changes: 65 additions & 0 deletions src/cortex-common/src/timeout.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}
40 changes: 28 additions & 12 deletions src/cortex-engine/src/tools/handlers/batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -45,6 +46,10 @@ pub struct BatchToolArgs {
/// Optional timeout in seconds for the entire batch (default: 300s).
#[serde(default)]
pub timeout_secs: Option<u64>,
/// 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<u64>,
}

/// Result of a single tool call within the batch.
Expand Down Expand Up @@ -158,7 +163,7 @@ impl BatchToolHandler {
&self,
calls: Vec<BatchToolCall>,
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())));
Expand All @@ -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;
Expand All @@ -202,19 +207,23 @@ impl BatchToolHandler {
duration_ms,
},
Ok(Err(e)) => BatchCallResult {
tool: tool_name,
tool: tool_name.clone(),
index,
success: false,
result: None,
error: Some(format!("Execution error: {}", e)),
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,
},
};
Expand Down Expand Up @@ -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);
Comment on lines +340 to +342
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timeout_secs parameter is still defined in BatchToolArgs and the tool definition but is no longer used in the implementation. Either remove it entirely (breaking change) or use it for an overall batch timeout wrapper around the parallel execution.

Suggested change
// 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);
// 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);
// Optionally enforce overall batch timeout if specified
let batch_timeout_secs = args.timeout_secs.unwrap_or(cortex_common::DEFAULT_BATCH_TIMEOUT_SECS);
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cortex-engine/src/tools/handlers/batch.rs
Line: 340:342

Comment:
`timeout_secs` parameter is still defined in `BatchToolArgs` and the tool definition but is no longer used in the implementation. Either remove it entirely (breaking change) or use it for an overall batch timeout wrapper around the parallel execution.

```suggestion
        // 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);
        
        // Optionally enforce overall batch timeout if specified
        let batch_timeout_secs = args.timeout_secs.unwrap_or(cortex_common::DEFAULT_BATCH_TIMEOUT_SECS);
```

How can I resolve this? If you propose a fix, please make it concise.


// 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
Expand Down Expand Up @@ -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
}
}
}),
Expand All @@ -409,6 +424,7 @@ pub async fn execute_batch(
})
.collect(),
timeout_secs: None,
tool_timeout_secs: None,
};

let arguments = serde_json::to_value(args)
Expand Down
5 changes: 3 additions & 2 deletions src/cortex-engine/src/tools/handlers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions src/cortex-engine/src/tools/unified_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions src/cortex-exec/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
39 changes: 25 additions & 14 deletions src/cortex-exec/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -267,7 +278,7 @@ impl ExecRunner {
/// Run the execution with full timeout enforcement.
pub async fn run(&mut self) -> Result<ExecResult, CortexError> {
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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
}
Expand Down
Loading