From f30f145c854b8a2e1874547adb615a56e302db8f Mon Sep 17 00:00:00 2001 From: "Dhanji R. Prasanna" Date: Tue, 13 Jan 2026 05:49:45 +0530 Subject: [PATCH] Fix UTF-8 panics and inconsistent retry logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix 7 UTF-8 byte slicing panics that crash on multi-byte characters: - acd.rs: extract_topic_from_text() [..50] slice - streaming.rs: log_stream_error() [..500] slice - tools/acd.rs: rehydrate message truncation [..2000] slice - history.rs: git commit message truncation [..69] slice - planner.rs: commit summary/description truncation [..69] slices - llm.rs: requirements summary line truncation [..117] slice - All now use chars().count() and chars().take(N).collect() for UTF-8 safe truncation - Fix inconsistent retry logic in task_execution.rs: - Previously only retried on Timeout errors - Now retries on ALL recoverable errors (rate limits, network, server errors, model busy, token limits, context length) - Added error-specific base delays (rate limit: 5s, server: 2s, etc.) - Added exponential backoff with ±20% jitter - Consistent with autonomous mode retry behavior --- Cargo.lock | 1 + crates/g3-cli/Cargo.toml | 1 + crates/g3-cli/src/task_execution.rs | 60 +++++++++++++++++++---------- crates/g3-core/src/acd.rs | 7 ++-- crates/g3-core/src/streaming.rs | 5 ++- crates/g3-core/src/tools/acd.rs | 5 ++- crates/g3-planner/src/history.rs | 5 ++- crates/g3-planner/src/llm.rs | 5 ++- crates/g3-planner/src/planner.rs | 10 +++-- 9 files changed, 64 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 91ed271..a8d46a4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1362,6 +1362,7 @@ dependencies = [ "hex", "indicatif", "once_cell", + "rand", "ratatui", "regex", "rustyline", diff --git a/crates/g3-cli/Cargo.toml b/crates/g3-cli/Cargo.toml index f6d6c3b..ac2dc53 100644 --- a/crates/g3-cli/Cargo.toml +++ b/crates/g3-cli/Cargo.toml @@ -31,6 +31,7 @@ termimad = "0.34.0" regex = "1.10" syntect = "5.3" once_cell = "1.19" +rand = "0.8" [dev-dependencies] tempfile = "3.8" diff --git a/crates/g3-cli/src/task_execution.rs b/crates/g3-cli/src/task_execution.rs index 86cae2d..d741845 100644 --- a/crates/g3-cli/src/task_execution.rs +++ b/crates/g3-cli/src/task_execution.rs @@ -8,10 +8,10 @@ use tracing::error; use crate::simple_output::SimpleOutput; -/// Maximum number of retry attempts for timeout errors -const MAX_TIMEOUT_RETRIES: u32 = 3; +/// Maximum number of retry attempts for recoverable errors +const MAX_RETRIES: u32 = 3; -/// Execute a task with retry logic for timeout errors. +/// Execute a task with retry logic for recoverable errors. pub async fn execute_task_with_retry( agent: &mut Agent, input: &str, @@ -61,29 +61,49 @@ pub async fn execute_task_with_retry( return; } - // Check if this is a timeout error that we should retry + // Check if this is a recoverable error that we should retry let error_type = classify_error(&e); - if matches!( - error_type, - ErrorType::Recoverable(RecoverableError::Timeout) - ) && attempt < MAX_TIMEOUT_RETRIES - { - // Calculate retry delay with exponential backoff - let delay_ms = 1000 * (2_u64.pow(attempt - 1)); - let delay = std::time::Duration::from_millis(delay_ms); + if let ErrorType::Recoverable(recoverable_error) = error_type { + if attempt < MAX_RETRIES { + // Calculate retry delay with exponential backoff + jitter + let base_delay_ms = match recoverable_error { + RecoverableError::RateLimit => 5000, // Rate limits need longer waits + RecoverableError::ServerError => 2000, + RecoverableError::NetworkError => 1000, + RecoverableError::Timeout => 1000, + RecoverableError::ModelBusy => 3000, + RecoverableError::TokenLimit => 1000, + RecoverableError::ContextLengthExceeded => 1000, + }; + let delay_ms = base_delay_ms * (2_u64.pow(attempt - 1)); + // Add jitter (±20%) + let jitter = (delay_ms as f64 * 0.2 * (rand::random::() - 0.5)) as i64; + let delay_ms = (delay_ms as i64 + jitter).max(100) as u64; + let delay = std::time::Duration::from_millis(delay_ms); - output.print(&format!( - "⏱️ Timeout error detected (attempt {}/{}). Retrying in {:?}...", - attempt, MAX_TIMEOUT_RETRIES, delay - )); + let error_name = match recoverable_error { + RecoverableError::RateLimit => "Rate limit", + RecoverableError::ServerError => "Server error", + RecoverableError::NetworkError => "Network error", + RecoverableError::Timeout => "Timeout", + RecoverableError::ModelBusy => "Model busy", + RecoverableError::TokenLimit => "Token limit", + RecoverableError::ContextLengthExceeded => "Context length", + }; - // Wait before retrying - tokio::time::sleep(delay).await; - continue; + output.print(&format!( + "⚠️ {} detected (attempt {}/{}). Retrying in {:.1}s...", + error_name, attempt, MAX_RETRIES, delay_ms as f64 / 1000.0 + )); + + // Wait before retrying + tokio::time::sleep(delay).await; + continue; + } } - // For non-timeout errors or after max retries + // For non-recoverable errors or after max retries handle_execution_error(&e, input, output, attempt); return; } diff --git a/crates/g3-core/src/acd.rs b/crates/g3-core/src/acd.rs index d349bc8..661711e 100644 --- a/crates/g3-core/src/acd.rs +++ b/crates/g3-core/src/acd.rs @@ -335,12 +335,13 @@ fn extract_topic_from_text(text: &str) -> String { let first_line = text.lines().next().unwrap_or(""); let cleaned = first_line.trim(); - if cleaned.len() <= 50 { + if cleaned.chars().count() <= 50 { cleaned.to_string() } else { - // Find a good break point - let truncated = &cleaned[..50]; + // Find a good break point (UTF-8 safe) + let truncated: String = cleaned.chars().take(50).collect(); if let Some(last_space) = truncated.rfind(' ') { + // last_space is a byte index into truncated, which is safe since truncated is a new String format!("{}...", &truncated[..last_space]) } else { format!("{}...", truncated) diff --git a/crates/g3-core/src/streaming.rs b/crates/g3-core/src/streaming.rs index e96ee77..18b7bae 100644 --- a/crates/g3-core/src/streaming.rs +++ b/crates/g3-core/src/streaming.rs @@ -200,8 +200,9 @@ pub fn log_stream_error( .rev() .find(|m| matches!(m.role, MessageRole::User)) { - let truncated = if last_user_msg.content.len() > 500 { - format!("{}... (truncated)", &last_user_msg.content[..500]) + let truncated = if last_user_msg.content.chars().count() > 500 { + let chars: String = last_user_msg.content.chars().take(500).collect(); + format!("{}... (truncated)", chars) } else { last_user_msg.content.clone() }; diff --git a/crates/g3-core/src/tools/acd.rs b/crates/g3-core/src/tools/acd.rs index b74c538..4d581e4 100644 --- a/crates/g3-core/src/tools/acd.rs +++ b/crates/g3-core/src/tools/acd.rs @@ -93,8 +93,9 @@ pub async fn execute_rehydrate( }; // Truncate very long messages for readability - let content = if msg.content.len() > 2000 { - format!("{}... [truncated, {} chars total]", &msg.content[..2000], msg.content.len()) + let content = if msg.content.chars().count() > 2000 { + let chars: String = msg.content.chars().take(2000).collect(); + format!("{}... [truncated, {} chars total]", chars, msg.content.chars().count()) } else { msg.content.clone() }; diff --git a/crates/g3-planner/src/history.rs b/crates/g3-planner/src/history.rs index eb83da8..bef05ee 100644 --- a/crates/g3-planner/src/history.rs +++ b/crates/g3-planner/src/history.rs @@ -137,8 +137,9 @@ pub fn write_completed_requirements( pub fn write_git_commit(plan_dir: &Path, message: &str) -> Result<()> { let timestamp = format_timestamp(); // Truncate message if too long for a single line - let truncated_message = if message.len() > 72 { - format!("{}...", &message[..69]) + let truncated_message = if message.chars().count() > 72 { + let chars: String = message.chars().take(69).collect(); + format!("{}...", chars) } else { message.to_string() }; diff --git a/crates/g3-planner/src/llm.rs b/crates/g3-planner/src/llm.rs index 1f40944..16691cd 100644 --- a/crates/g3-planner/src/llm.rs +++ b/crates/g3-planner/src/llm.rs @@ -138,8 +138,9 @@ pub async fn generate_requirements_summary( .lines() .take(5) .map(|line| { - if line.len() > 120 { - format!("{}...", &line[..117]) + if line.chars().count() > 120 { + let chars: String = line.chars().take(117).collect(); + format!("{}...", chars) } else { line.to_string() } diff --git a/crates/g3-planner/src/planner.rs b/crates/g3-planner/src/planner.rs index 597f517..83e3232 100644 --- a/crates/g3-planner/src/planner.rs +++ b/crates/g3-planner/src/planner.rs @@ -519,8 +519,9 @@ pub fn parse_commit_message(response: &str) -> (String, String) { } // Ensure summary is max 72 chars - if summary.len() > 72 { - summary = format!("{}...", &summary[..69]); + if summary.chars().count() > 72 { + let chars: String = summary.chars().take(69).collect(); + summary = format!("{}...", chars); } // Ensure description lines are max 72 chars @@ -528,8 +529,9 @@ pub fn parse_commit_message(response: &str) -> (String, String) { .lines() .take(10) // Max 10 lines .map(|line| { - if line.len() > 72 { - format!("{}...", &line[..69]) + if line.chars().count() > 72 { + let chars: String = line.chars().take(69).collect(); + format!("{}...", chars) } else { line.to_string() }