Skip to content
Open
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: 3 additions & 3 deletions src/cortex-app-server/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl Claims {
pub fn new(user_id: impl Into<String>, expiry_seconds: u64) -> Self {
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.unwrap_or_default()
.as_secs();

Self {
Expand Down Expand Up @@ -75,7 +75,7 @@ impl Claims {
pub fn is_expired(&self) -> bool {
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.unwrap_or_default()
.as_secs();
self.exp < now
}
Expand Down Expand Up @@ -187,7 +187,7 @@ impl AuthService {
pub async fn cleanup_revoked_tokens(&self) {
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.unwrap_or_default()
.as_secs();

let mut revoked = self.revoked_tokens.write().await;
Expand Down
13 changes: 12 additions & 1 deletion src/cortex-app-server/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,18 @@ pub struct ServerConfig {
pub max_body_size: usize,

/// Request timeout in seconds (applies to full request lifecycle).
///
/// See `cortex_common::http_client` module documentation for the complete
/// timeout hierarchy across Cortex services.
#[serde(default = "default_request_timeout")]
pub request_timeout: u64,

/// Read timeout for individual chunks in seconds.
/// Applies to chunked transfer encoding to prevent indefinite hangs
/// when clients disconnect without sending the terminal chunk.
///
/// See `cortex_common::http_client` module documentation for the complete
/// timeout hierarchy across Cortex services.
#[serde(default = "default_read_timeout")]
pub read_timeout: u64,

Expand All @@ -71,12 +77,17 @@ pub struct ServerConfig {
pub cors_origins: Vec<String>,

/// Graceful shutdown timeout in seconds.
///
/// See `cortex_common::http_client` module documentation for the complete
/// timeout hierarchy across Cortex services.
#[serde(default = "default_shutdown_timeout")]
pub shutdown_timeout: u64,
}

fn default_shutdown_timeout() -> u64 {
30 // 30 seconds for graceful shutdown
// 30 seconds for graceful shutdown
// See cortex_common::http_client for timeout hierarchy documentation
30
}

fn default_listen_addr() -> String {
Expand Down
3 changes: 0 additions & 3 deletions src/cortex-app-server/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ pub struct StoredToolCall {

/// Session storage manager.
pub struct SessionStorage {
#[allow(dead_code)]
base_dir: PathBuf,
sessions_dir: PathBuf,
history_dir: PathBuf,
}
Expand All @@ -66,7 +64,6 @@ impl SessionStorage {
info!("Session storage initialized at {:?}", base_dir);

Ok(Self {
base_dir,
sessions_dir,
history_dir,
})
Expand Down
4 changes: 2 additions & 2 deletions src/cortex-app-server/src/streaming.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,10 @@ async fn session_events_stream(
serde_json::to_string(&StreamEvent::Ping {
timestamp: std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.unwrap_or_default()
.as_secs(),
})
.unwrap(),
.unwrap_or_default(),
)))
.await;

Expand Down
11 changes: 0 additions & 11 deletions src/cortex-apply-patch/src/hunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,6 @@ pub struct SearchReplace {
pub search: String,
/// The text to replace with.
pub replace: String,
/// Replace all occurrences (true) or just the first (false).
#[allow(dead_code)]
pub replace_all: bool,
}

impl SearchReplace {
Expand All @@ -266,16 +263,8 @@ impl SearchReplace {
path: path.into(),
search: search.into(),
replace: replace.into(),
replace_all: false,
}
}

/// Set whether to replace all occurrences.
#[allow(dead_code)]
pub fn with_replace_all(mut self, replace_all: bool) -> Self {
self.replace_all = replace_all;
self
}
}

#[cfg(test)]
Expand Down
49 changes: 27 additions & 22 deletions src/cortex-common/src/config_substitution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,22 @@

use regex::Regex;
use std::path::PathBuf;
use std::sync::LazyLock;
use thiserror::Error;

/// Static regex for environment variable substitution: {env:VAR} or {env:VAR:default}
/// Group 1: variable name
/// Group 2: optional default value (after second colon)
static ENV_REGEX: LazyLock<Regex> = LazyLock::new(|| {
Regex::new(r"\{env:([^:}]+)(?::([^}]*))?\}").expect("env regex pattern is valid and tested")
});

/// Static regex for file content substitution: {file:path}
/// Group 1: file path
static FILE_REGEX: LazyLock<Regex> = LazyLock::new(|| {
Regex::new(r"\{file:([^}]+)\}").expect("file regex pattern is valid and tested")
});

/// Errors that can occur during configuration substitution.
#[derive(Debug, Error)]
pub enum SubstitutionError {
Expand Down Expand Up @@ -42,11 +56,13 @@ pub enum SubstitutionError {
///
/// Handles replacement of `{env:...}` and `{file:...}` placeholders
/// in configuration strings.
///
/// This struct uses statically initialized regex patterns via `LazyLock`,
/// making regex compilation a one-time cost shared across all instances.
pub struct ConfigSubstitution {
/// Regex for environment variable substitution: {env:VAR} or {env:VAR:default}
env_regex: Regex,
/// Regex for file content substitution: {file:path}
file_regex: Regex,
// This struct is kept for API compatibility.
// Regex patterns are now static module-level constants.
_private: (),
}

impl Default for ConfigSubstitution {
Expand All @@ -56,22 +72,13 @@ impl Default for ConfigSubstitution {
}

impl ConfigSubstitution {
/// Creates a new `ConfigSubstitution` instance with compiled regex patterns.
/// Creates a new `ConfigSubstitution` instance.
///
/// The regex patterns are statically initialized on first use,
/// so creating multiple instances has no additional cost.
#[must_use]
pub fn new() -> Self {
Self {
// Matches {env:VAR_NAME} or {env:VAR_NAME:default_value}
// Group 1: variable name
// Group 2: optional default value (after second colon)
env_regex: Regex::new(r"\{env:([^:}]+)(?::([^}]*))?\}").unwrap_or_else(|e| {
panic!("Failed to compile env regex: {e}");
}),
// Matches {file:path}
// Group 1: file path
file_regex: Regex::new(r"\{file:([^}]+)\}").unwrap_or_else(|e| {
panic!("Failed to compile file regex: {e}");
}),
}
Self { _private: () }
}

/// Substitutes all variables in a string.
Expand Down Expand Up @@ -109,8 +116,7 @@ impl ConfigSubstitution {
let mut error: Option<SubstitutionError> = None;

// Collect all matches first to avoid borrowing issues
let matches: Vec<_> = self
.env_regex
let matches: Vec<_> = ENV_REGEX
.captures_iter(input)
.map(|cap| {
let full_match = cap.get(0).map(|m| m.as_str().to_string());
Expand Down Expand Up @@ -155,8 +161,7 @@ impl ConfigSubstitution {
let mut error: Option<SubstitutionError> = None;

// Collect all matches first
let matches: Vec<_> = self
.file_regex
let matches: Vec<_> = FILE_REGEX
.captures_iter(input)
.map(|cap| {
let full_match = cap.get(0).map(|m| m.as_str().to_string());
Expand Down
84 changes: 79 additions & 5 deletions src/cortex-common/src/http_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,99 @@
//!
//! DNS caching is configured with reasonable TTL to allow failover and load
//! balancer updates (#2177).
//!
//! # Timeout Configuration Guide
//!
//! This section documents the timeout hierarchy across the Cortex codebase. Use this
//! as a reference when configuring timeouts for new features or debugging timeout issues.
//!
//! ## Timeout Hierarchy
//!
//! | Use Case | Timeout | Constant/Location | Rationale |
//! |-----------------------------|---------|--------------------------------------------|-----------------------------------------|
//! | Health checks | 5s | `HEALTH_CHECK_TIMEOUT` (this module) | Quick validation of service status |
//! | Standard HTTP requests | 30s | `DEFAULT_TIMEOUT` (this module) | Normal API calls with reasonable margin |
//! | Per-chunk read (streaming) | 30s | `read_timeout` (cortex-app-server/config) | Individual chunk timeout during stream |
//! | Pool idle timeout | 60s | `POOL_IDLE_TIMEOUT` (this module) | DNS re-resolution for failover |
//! | LLM Request (non-streaming) | 120s | `DEFAULT_REQUEST_TIMEOUT_SECS` (cortex-exec/runner) | Model inference takes time |
//! | LLM Streaming total | 300s | `STREAMING_TIMEOUT` (this module) | Long-running streaming responses |
//! | Server request lifecycle | 300s | `request_timeout` (cortex-app-server/config) | Full HTTP request/response cycle |
//! | Entire exec session | 600s | `DEFAULT_TIMEOUT_SECS` (cortex-exec/runner) | Multi-turn conversation limit |
//! | Graceful shutdown | 30s | `shutdown_timeout` (cortex-app-server/config) | Time for cleanup on shutdown |
//!
//! ## Module-Specific Timeouts
//!
//! ### cortex-common (this module)
//! - `DEFAULT_TIMEOUT` (30s): Use for standard API calls.
//! - `STREAMING_TIMEOUT` (300s): Use for LLM streaming endpoints.
//! - `HEALTH_CHECK_TIMEOUT` (5s): Use for health/readiness checks.
//! - `POOL_IDLE_TIMEOUT` (60s): Connection pool cleanup for DNS freshness.
//!
//! ### cortex-exec (runner.rs)
//! - `DEFAULT_TIMEOUT_SECS` (600s): Maximum duration for entire exec session.
//! - `DEFAULT_REQUEST_TIMEOUT_SECS` (120s): Single LLM request timeout.
//!
//! ### cortex-app-server (config.rs)
//! - `request_timeout` (300s): Full request lifecycle timeout.
//! - `read_timeout` (30s): Per-chunk timeout for streaming reads.
//! - `shutdown_timeout` (30s): Graceful shutdown duration.
//!
//! ### cortex-engine (api_client.rs)
//! - Re-exports constants from this module for consistency.
//!
//! ## Recommendations
//!
//! When adding new timeout configurations:
//! 1. Use constants from this module when possible for consistency.
//! 2. Document any new timeout constants with their rationale.
//! 3. Consider the timeout hierarchy - inner timeouts should be shorter than outer ones.
//! 4. For LLM operations, use longer timeouts (120s-300s) to accommodate model inference.
//! 5. For health checks and quick validations, use short timeouts (5s-10s).

use reqwest::Client;
use std::time::Duration;

// ============================================================
// Timeout Configuration Constants
// ============================================================
//
// Timeout Hierarchy Documentation
// ===============================
//
// This module defines the authoritative timeout constants for HTTP operations.
// Other modules should reference these constants or document deviations.
//
// Timeout Categories:
// - Request timeouts: Total time for a complete request/response cycle
// - Connection timeouts: Time to establish TCP connection
// - Read timeouts: Time to receive response data
// - Pool timeouts: How long idle connections stay in pool
//
// Recommended Naming Convention:
// - Constants: SCREAMING_SNAKE_CASE with Duration type
// - Config fields: snake_case with _secs suffix for u64 values
//
// ============================================================

/// User-Agent string for all HTTP requests
pub const USER_AGENT: &str = concat!("cortex-cli/", env!("CARGO_PKG_VERSION"));

/// Default timeout for standard API requests (30 seconds)
/// Default timeout for standard HTTP requests (30 seconds).
/// Used for non-streaming API calls, health checks with retries, etc.
pub const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30);

/// Extended timeout for LLM streaming requests (5 minutes)
/// Timeout for streaming HTTP requests (5 minutes).
/// Longer duration to accommodate LLM inference time.
pub const STREAMING_TIMEOUT: Duration = Duration::from_secs(300);

/// Short timeout for health checks (5 seconds)
/// Timeout for health check requests (5 seconds).
/// Short timeout since health endpoints should respond quickly.
pub const HEALTH_CHECK_TIMEOUT: Duration = Duration::from_secs(5);

/// Connection pool idle timeout to ensure DNS is re-resolved periodically.
/// Idle timeout for connection pool (60 seconds).
/// Connections are closed after being idle for this duration
/// to allow DNS re-resolution for services behind load balancers.
/// This helps with failover scenarios where DNS records change (#2177).
/// Set to 60 seconds to balance between performance and DNS freshness.
pub const POOL_IDLE_TIMEOUT: Duration = Duration::from_secs(60);

/// Creates an HTTP client with default configuration (30s timeout).
Expand Down
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
Loading