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
148 changes: 142 additions & 6 deletions src/cortex-agents/src/mention.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,46 @@
use regex::Regex;
use std::sync::LazyLock;

/// Safely get the string slice up to the given byte position.
///
/// Returns the slice `&text[..pos]` if `pos` is at a valid UTF-8 character boundary.
/// If `pos` is inside a multi-byte character, finds the nearest valid boundary
/// by searching backwards.
fn safe_slice_up_to(text: &str, pos: usize) -> &str {
if pos >= text.len() {
return text;
}
if text.is_char_boundary(pos) {
return &text[..pos];
}
// Find the nearest valid boundary by searching backwards
let mut valid_pos = pos;
while valid_pos > 0 && !text.is_char_boundary(valid_pos) {
valid_pos -= 1;
}
&text[..valid_pos]
}

/// Safely get the string slice from the given byte position to the end.
///
/// Returns the slice `&text[pos..]` if `pos` is at a valid UTF-8 character boundary.
/// If `pos` is inside a multi-byte character, finds the nearest valid boundary
/// by searching forwards.
fn safe_slice_from(text: &str, pos: usize) -> &str {
if pos >= text.len() {
return "";
}
if text.is_char_boundary(pos) {
return &text[pos..];
}
// Find the nearest valid boundary by searching forwards
let mut valid_pos = pos;
while valid_pos < text.len() && !text.is_char_boundary(valid_pos) {
valid_pos += 1;
}
&text[valid_pos..]
}

/// A parsed agent mention from user input.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct AgentMention {
Expand Down Expand Up @@ -108,10 +148,10 @@ pub fn extract_mention_and_text(
) -> Option<(AgentMention, String)> {
let mention = find_first_valid_mention(text, valid_agents)?;

// Remove the mention from text
// Remove the mention from text, using safe slicing for UTF-8 boundaries
let mut remaining = String::with_capacity(text.len());
remaining.push_str(&text[..mention.start]);
remaining.push_str(&text[mention.end..]);
remaining.push_str(safe_slice_up_to(text, mention.start));
remaining.push_str(safe_slice_from(text, mention.end));

// Trim and normalize whitespace
let remaining = remaining.trim().to_string();
Expand All @@ -123,7 +163,8 @@ pub fn extract_mention_and_text(
pub fn starts_with_mention(text: &str, valid_agents: &[&str]) -> bool {
let text = text.trim();
if let Some(mention) = find_first_valid_mention(text, valid_agents) {
mention.start == 0 || text[..mention.start].trim().is_empty()
// Use safe slicing to handle UTF-8 boundaries
mention.start == 0 || safe_slice_up_to(text, mention.start).trim().is_empty()
} else {
false
}
Expand Down Expand Up @@ -196,8 +237,8 @@ pub fn parse_message_for_agent(text: &str, valid_agents: &[&str]) -> ParsedAgent

// Check if message starts with @agent
if let Some((mention, remaining)) = extract_mention_and_text(text, valid_agents) {
// Only trigger if mention is at the start
if mention.start == 0 || text[..mention.start].trim().is_empty() {
// Only trigger if mention is at the start, using safe slicing for UTF-8 boundaries
if mention.start == 0 || safe_slice_up_to(text, mention.start).trim().is_empty() {
return ParsedAgentMessage::for_agent(mention.agent_name, remaining, text.to_string());
}
}
Expand Down Expand Up @@ -318,4 +359,99 @@ mod tests {
assert_eq!(mentions[0].agent_name, "my-agent");
assert_eq!(mentions[1].agent_name, "my_agent");
}

// UTF-8 boundary safety tests
#[test]
fn test_safe_slice_up_to_ascii() {
let text = "hello world";
assert_eq!(safe_slice_up_to(text, 5), "hello");
assert_eq!(safe_slice_up_to(text, 0), "");
assert_eq!(safe_slice_up_to(text, 100), "hello world");
}

#[test]
fn test_safe_slice_up_to_multibyte() {
// "こんにちは" - each character is 3 bytes
let text = "こんにちは";
assert_eq!(safe_slice_up_to(text, 3), "こ"); // Valid boundary
assert_eq!(safe_slice_up_to(text, 6), "こん"); // Valid boundary
// Position 4 is inside the second character, should return "こ"
assert_eq!(safe_slice_up_to(text, 4), "こ");
assert_eq!(safe_slice_up_to(text, 5), "こ");
}

#[test]
fn test_safe_slice_from_multibyte() {
let text = "こんにちは";
assert_eq!(safe_slice_from(text, 3), "んにちは"); // Valid boundary
// Position 4 is inside second character, should skip to position 6
assert_eq!(safe_slice_from(text, 4), "にちは");
assert_eq!(safe_slice_from(text, 5), "にちは");
}

#[test]
fn test_extract_mention_with_multibyte_prefix() {
let valid = vec!["general"];

// Multi-byte characters before mention
let result = extract_mention_and_text("日本語 @general search files", &valid);
assert!(result.is_some());
let (mention, remaining) = result.unwrap();
assert_eq!(mention.agent_name, "general");
// The prefix should be preserved without panicking
assert!(remaining.contains("search files"));
}

#[test]
fn test_starts_with_mention_multibyte() {
let valid = vec!["general"];

// Whitespace with multi-byte characters should not cause panic
assert!(starts_with_mention(" @general task", &valid));

// Multi-byte characters before mention - should return false, not panic
assert!(!starts_with_mention("日本語 @general task", &valid));
}

#[test]
fn test_parse_message_for_agent_multibyte() {
let valid = vec!["general"];

// Multi-byte prefix - should not panic
let parsed = parse_message_for_agent("日本語 @general find files", &valid);
// Since mention is not at the start, should not invoke task
assert!(!parsed.should_invoke_task);

// Multi-byte in the prompt (after mention)
let parsed = parse_message_for_agent("@general 日本語を検索", &valid);
assert!(parsed.should_invoke_task);
assert_eq!(parsed.agent, Some("general".to_string()));
assert_eq!(parsed.prompt, "日本語を検索");
}

#[test]
fn test_extract_mention_with_emoji() {
let valid = vec!["general"];

// Emojis are 4 bytes each
let result = extract_mention_and_text("🎉 @general celebrate", &valid);
assert!(result.is_some());
let (mention, remaining) = result.unwrap();
assert_eq!(mention.agent_name, "general");
assert!(remaining.contains("celebrate"));
}

#[test]
fn test_mixed_multibyte_and_ascii() {
let valid = vec!["general"];

// Mix of ASCII, CJK, and emoji
let text = "Hello 世界 🌍 @general search for 日本語";
let result = extract_mention_and_text(text, &valid);
assert!(result.is_some());
let (mention, remaining) = result.unwrap();
assert_eq!(mention.agent_name, "general");
// Should not panic and produce valid output
assert!(!remaining.is_empty());
}
}
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
10 changes: 10 additions & 0 deletions 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,16 @@ 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
// See cortex_common::http_client for timeout hierarchy documentation
}

fn default_listen_addr() -> String {
Expand Down
3 changes: 2 additions & 1 deletion src/cortex-app-server/src/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ pub async fn request_id_middleware(mut request: Request, next: Next) -> Response
let mut response = next.run(request).await;
response.headers_mut().insert(
REQUEST_ID_HEADER,
HeaderValue::from_str(&request_id).unwrap(),
HeaderValue::from_str(&request_id)
.unwrap_or_else(|_| HeaderValue::from_static("invalid-request-id")),
);

response
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
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
48 changes: 48 additions & 0 deletions src/cortex-common/src/http_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,54 @@
//!
//! 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;
Expand Down
Loading
Loading