Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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