From b483467e1649d1bd39b6721deb81ae90b1144001 Mon Sep 17 00:00:00 2001 From: Alex Holmberg Date: Sun, 11 Jan 2026 01:19:11 +0100 Subject: [PATCH] feat: small fixes, truncation for docker output, default bedrock model fix, and lastly shell error fixed --- src/agent/session.rs | 2 +- src/agent/ui/hooks.rs | 127 +++++++++++++---------- src/agent/ui/shell_output.rs | 112 ++++++++++++++++---- src/agent/ui/tool_display.rs | 19 ++-- src/analyzer/syncable-cli.code-workspace | 8 ++ 5 files changed, 186 insertions(+), 82 deletions(-) create mode 100644 src/analyzer/syncable-cli.code-workspace diff --git a/src/agent/session.rs b/src/agent/session.rs index 57cdbf5f..a5b276f2 100644 --- a/src/agent/session.rs +++ b/src/agent/session.rs @@ -186,7 +186,7 @@ impl ChatSession { let default_model = match provider { ProviderType::OpenAI => "gpt-5.2".to_string(), ProviderType::Anthropic => "claude-sonnet-4-5-20250929".to_string(), - ProviderType::Bedrock => "global.anthropic.claude-sonnet-4-5-20250929-v1:0".to_string(), + ProviderType::Bedrock => "global.anthropic.claude-sonnet-4-20250514-v1:0".to_string(), }; Self { diff --git a/src/agent/ui/hooks.rs b/src/agent/ui/hooks.rs index af7a6497..5e3775ea 100644 --- a/src/agent/ui/hooks.rs +++ b/src/agent/ui/hooks.rs @@ -19,6 +19,19 @@ use tokio::sync::Mutex; /// Maximum lines to show in preview before collapsing const PREVIEW_LINES: usize = 4; +/// Safely truncate a string to a maximum character count, handling UTF-8 properly. +/// Adds "..." suffix when truncation occurs. +fn truncate_safe(s: &str, max_chars: usize) -> String { + let char_count = s.chars().count(); + if char_count <= max_chars { + s.to_string() + } else { + let truncate_to = max_chars.saturating_sub(3); + let truncated: String = s.chars().take(truncate_to).collect(); + format!("{}...", truncated) + } +} + /// Tool call state with full output for expansion #[derive(Debug, Clone)] pub struct ToolCallState { @@ -620,6 +633,34 @@ fn print_tool_result(name: &str, args: &str, result: &str) -> (bool, Vec } }); + // If parsing failed, check if it's a tool error message + // Tool errors come through as plain strings like "Shell error: ..." + let parsed = if parsed.is_err() && !result.is_empty() { + // Check for common error patterns + let is_tool_error = result.contains("error:") + || result.contains("Error:") + || result.starts_with("Shell error") + || result.starts_with("Toolset error") + || result.starts_with("ToolCallError"); + + if is_tool_error { + // Wrap the error message in a JSON structure so formatters can handle it + let clean_msg = result + .replace("Toolset error: ", "") + .replace("ToolCallError: ", "") + .replace("Shell error: ", ""); + Ok(serde_json::json!({ + "error": true, + "message": clean_msg, + "success": false + })) + } else { + parsed + } + } else { + parsed + }; + // Format output based on tool type let (status_ok, output_lines) = match name { "shell" => format_shell_result(&parsed), @@ -790,6 +831,19 @@ fn format_shell_result( parsed: &Result, ) -> (bool, Vec) { if let Ok(v) = parsed { + // Check if this is an error message (from tool error or blocked command) + if let Some(error_msg) = v.get("message").and_then(|m| m.as_str()) { + if v.get("error").and_then(|e| e.as_bool()).unwrap_or(false) { + return (false, vec![error_msg.to_string()]); + } + } + + // Check for cancelled or blocked operations (plan mode, user cancel) + if v.get("cancelled").and_then(|c| c.as_bool()).unwrap_or(false) { + let reason = v.get("reason").and_then(|r| r.as_str()).unwrap_or("cancelled"); + return (false, vec![reason.to_string()]); + } + let success = v.get("success").and_then(|s| s.as_bool()).unwrap_or(false); let stdout = v.get("stdout").and_then(|s| s.as_str()).unwrap_or(""); let stderr = v.get("stderr").and_then(|s| s.as_str()).unwrap_or(""); @@ -1185,11 +1239,7 @@ fn format_hadolint_result( if let Some(quick_fixes) = v.get("quick_fixes").and_then(|q| q.as_array()) && let Some(first_fix) = quick_fixes.first().and_then(|f| f.as_str()) { - let truncated = if first_fix.len() > 70 { - format!("{}...", &first_fix[..67]) - } else { - first_fix.to_string() - }; + let truncated = truncate_safe(first_fix, 70); lines.push(format!( "{} → Fix: {}{}", ansi::INFO_BLUE, @@ -1233,11 +1283,7 @@ fn format_hadolint_issue(issue: &serde_json::Value, icon: &str, color: &str) -> }; // Truncate message - let msg_display = if message.len() > 50 { - format!("{}...", &message[..47]) - } else { - message.to_string() - }; + let msg_display = truncate_safe(message, 50); format!( "{}{} L{}:{} {}{}[{}]{} {} {}", @@ -1284,11 +1330,7 @@ fn format_kubelint_result( )); for (i, err) in errors.iter().take(3).enumerate() { if let Some(err_str) = err.as_str() { - let truncated = if err_str.len() > 70 { - format!("{}...", &err_str[..67]) - } else { - err_str.to_string() - }; + let truncated = truncate_safe(err_str, 70); lines.push(format!( "{} {} {}{}", ansi::HIGH, @@ -1420,11 +1462,7 @@ fn format_kubelint_result( if let Some(quick_fixes) = v.get("quick_fixes").and_then(|q| q.as_array()) && let Some(first_fix) = quick_fixes.first().and_then(|f| f.as_str()) { - let truncated = if first_fix.len() > 70 { - format!("{}...", &first_fix[..67]) - } else { - first_fix.to_string() - }; + let truncated = truncate_safe(first_fix, 70); lines.push(format!( "{} → Fix: {}{}", ansi::INFO_BLUE, @@ -1450,8 +1488,6 @@ fn format_kubelint_result( (false, vec!["kubelint analysis complete".to_string()]) } } - -/// Format a single kubelint issue for display fn format_kubelint_issue(issue: &serde_json::Value, icon: &str, color: &str) -> String { let check = issue.get("check").and_then(|c| c.as_str()).unwrap_or("?"); let message = issue.get("message").and_then(|m| m.as_str()).unwrap_or("?"); @@ -1468,11 +1504,7 @@ fn format_kubelint_issue(issue: &serde_json::Value, icon: &str, color: &str) -> }; // Truncate message - let msg_display = if message.len() > 50 { - format!("{}...", &message[..47]) - } else { - message.to_string() - }; + let msg_display = truncate_safe(message, 50); format!( "{}{} L{}:{} {}{}[{}]{} {} {}", @@ -1519,11 +1551,7 @@ fn format_helmlint_result( )); for (i, err) in errors.iter().take(3).enumerate() { if let Some(err_str) = err.as_str() { - let truncated = if err_str.len() > 70 { - format!("{}...", &err_str[..67]) - } else { - err_str.to_string() - }; + let truncated = truncate_safe(err_str, 70); lines.push(format!( "{} {} {}{}", ansi::HIGH, @@ -1655,11 +1683,7 @@ fn format_helmlint_result( if let Some(quick_fixes) = v.get("quick_fixes").and_then(|q| q.as_array()) && let Some(first_fix) = quick_fixes.first().and_then(|f| f.as_str()) { - let truncated = if first_fix.len() > 70 { - format!("{}...", &first_fix[..67]) - } else { - first_fix.to_string() - }; + let truncated = truncate_safe(first_fix, 70); lines.push(format!( "{} → Fix: {}{}", ansi::INFO_BLUE, @@ -1704,18 +1728,15 @@ fn format_helmlint_issue(issue: &serde_json::Value, icon: &str, color: &str) -> }; // Short file name - let file_short = if file.len() > 20 { - format!("...{}", &file[file.len().saturating_sub(17)..]) + let file_short = if file.chars().count() > 20 { + let skip = file.chars().count().saturating_sub(17); + format!("...{}", file.chars().skip(skip).collect::()) } else { file.to_string() }; // Truncate message - let msg_display = if message.len() > 40 { - format!("{}...", &message[..37]) - } else { - message.to_string() - }; + let msg_display = truncate_safe(message, 40); format!( "{}{} {}:{}:{} {}{}[{}]{} {} {}", @@ -1923,11 +1944,7 @@ fn format_result_preview(result: &serde_json::Value) -> String { .and_then(|v| v.as_str()) .unwrap_or(""); - let detail_short = if detail.len() > 40 { - format!("{}...", &detail[..37]) - } else { - detail.to_string() - }; + let detail_short = truncate_safe(detail, 40); if detail_short.is_empty() { name.to_string() @@ -1969,8 +1986,10 @@ fn tool_to_focus(tool_name: &str, args: &str) -> Option { "read_file" | "write_file" => { parsed.get("path").and_then(|p| p.as_str()).map(|p| { // Shorten long paths - if p.len() > 50 { - format!("...{}", &p[p.len().saturating_sub(47)..]) + let char_count = p.chars().count(); + if char_count > 50 { + let skip = char_count.saturating_sub(47); + format!("...{}", p.chars().skip(skip).collect::()) } else { p.to_string() } @@ -1982,11 +2001,7 @@ fn tool_to_focus(tool_name: &str, args: &str) -> Option { .map(|p| p.to_string()), "shell" => parsed.get("command").and_then(|c| c.as_str()).map(|cmd| { // Truncate long commands - if cmd.len() > 60 { - format!("{}...", &cmd[..57]) - } else { - cmd.to_string() - } + truncate_safe(cmd, 60) }), "hadolint" | "dclint" | "kubelint" | "helmlint" => parsed .get("path") diff --git a/src/agent/ui/shell_output.rs b/src/agent/ui/shell_output.rs index 667db7b0..9f058b6f 100644 --- a/src/agent/ui/shell_output.rs +++ b/src/agent/ui/shell_output.rs @@ -76,15 +76,11 @@ impl StreamingShellOutput { let elapsed = self.format_elapsed(); let timeout = self.format_timeout(); - // Truncate command if needed + // Truncate command if needed (using safe UTF-8 truncation) let term_width = term_size::dimensions().map(|(w, _)| w).unwrap_or(80); let prefix_len = 2 + timeout.len() + elapsed.len() + 10; // "● Bash(" + ") " + times let max_cmd_len = term_width.saturating_sub(prefix_len); - let cmd_display = if self.command.len() > max_cmd_len { - format!("{}...", &self.command[..max_cmd_len.saturating_sub(3)]) - } else { - self.command.clone() - }; + let cmd_display = truncate_safe(&self.command, max_cmd_len); print!( "{} {}({}) {} ({})", @@ -105,12 +101,8 @@ impl StreamingShellOutput { let is_last = i == self.lines.len() - 1; let prefix = if is_last { "└" } else { "│" }; - // Truncate line if needed - let display = if line.len() > content_width { - format!("{}...", &line[..content_width.saturating_sub(3)]) - } else { - line.clone() - }; + // Truncate line if needed (using safe UTF-8 truncation) + let display = truncate_safe(line, content_width); println!(" {} {}", prefix.dimmed(), display); } @@ -188,14 +180,10 @@ impl StreamingShellOutput { let elapsed = self.format_elapsed(); let status_icon = if success { "✓" } else { "✗" }; - // Final header + // Final header (using safe UTF-8 truncation) let term_width = term_size::dimensions().map(|(w, _)| w).unwrap_or(80); let max_cmd_len = term_width.saturating_sub(30); - let cmd_display = if self.command.len() > max_cmd_len { - format!("{}...", &self.command[..max_cmd_len.saturating_sub(3)]) - } else { - self.command.clone() - }; + let cmd_display = truncate_safe(&self.command, max_cmd_len); let exit_info = match exit_code { Some(code) if code != 0 => format!(" (exit {})", code), @@ -267,6 +255,39 @@ fn strip_ansi_codes(s: &str) -> String { result } +/// Safely truncate a string to a maximum visual width, handling UTF-8 properly. +/// Adds "..." suffix when truncation occurs. +/// This prevents panics from slicing multi-byte UTF-8 characters. +fn truncate_safe(s: &str, max_width: usize) -> String { + // Strip ANSI codes first to get accurate visual width + let stripped = strip_ansi_codes(s); + + // Calculate visual width (count characters, not bytes) + let visual_len: usize = stripped.chars().count(); + + if visual_len <= max_width { + return s.to_string(); + } + + // Need to truncate - work with stripped version + // Reserve space for "..." + let truncate_to = max_width.saturating_sub(3); + + let mut result = String::new(); + let mut char_count = 0; + + for ch in stripped.chars() { + if char_count >= truncate_to { + result.push_str("..."); + break; + } + result.push(ch); + char_count += 1; + } + + result +} + #[cfg(test)] mod tests { use super::*; @@ -277,6 +298,51 @@ mod tests { assert_eq!(strip_ansi_codes(input), "green text"); } + #[test] + fn test_truncate_safe_ascii() { + // Basic ASCII truncation + assert_eq!(truncate_safe("hello world", 8), "hello..."); + assert_eq!(truncate_safe("short", 10), "short"); + assert_eq!(truncate_safe("exactly10!", 10), "exactly10!"); + } + + #[test] + fn test_truncate_safe_utf8_box_drawing() { + // Box drawing characters (multi-byte UTF-8) - the exact case that caused the panic + let box_line = "╭ Warning ──────────────────────────────────╮"; + // Should NOT panic and should truncate properly + let result = truncate_safe(box_line, 20); + assert!(result.ends_with("...")); + assert!(result.chars().count() <= 20); + } + + #[test] + fn test_truncate_safe_utf8_emoji() { + // Emoji (multi-byte UTF-8) + let emoji_str = "🚀 Building project 📦 with dependencies 🔧"; + let result = truncate_safe(emoji_str, 15); + assert!(result.ends_with("...")); + // Should not panic + } + + #[test] + fn test_truncate_safe_mixed_content() { + // Mixed ASCII and multi-byte characters + let mixed = "#9 3.304 ╭ Warning ───"; + let result = truncate_safe(mixed, 15); + assert!(result.ends_with("...")); + assert!(result.chars().count() <= 15); + } + + #[test] + fn test_truncate_safe_no_truncation_needed() { + let short = "hello"; + assert_eq!(truncate_safe(short, 100), "hello"); + + let exact = "12345"; + assert_eq!(truncate_safe(exact, 5), "12345"); + } + #[test] fn test_streaming_output_buffer() { let mut stream = StreamingShellOutput::new("test", 60); @@ -290,4 +356,14 @@ mod tests { } assert_eq!(stream.lines.len(), DEFAULT_MAX_LINES); } + + #[test] + fn test_streaming_output_with_utf8_content() { + // Ensure the buffer doesn't panic with UTF-8 content + let mut stream = StreamingShellOutput::new("docker build", 60); + stream.push_line("╭ Warning ────────────────╮"); + stream.push_line("│ This is a warning message │"); + stream.push_line("╰────────────────────────────╯"); + assert_eq!(stream.lines.len(), 3); + } } diff --git a/src/agent/ui/tool_display.rs b/src/agent/ui/tool_display.rs index 2110c547..d276e978 100644 --- a/src/agent/ui/tool_display.rs +++ b/src/agent/ui/tool_display.rs @@ -135,8 +135,9 @@ impl ToolCallDisplay { /// Print a tool call result (for verbose output) pub fn print_result(name: &str, result: &str, truncate: bool) { - let display_result = if truncate && result.len() > 200 { - format!("{}... (truncated)", &result[..200]) + let display_result = if truncate && result.chars().count() > 200 { + let truncated: String = result.chars().take(200).collect(); + format!("{}... (truncated)", truncated) } else { result.to_string() }; @@ -252,8 +253,9 @@ impl ForgeToolDisplay { let line_count = s.lines().count(); if line_count > 1 { format!("<{} lines>", line_count) - } else if s.len() > 50 { - format!("{}...", &s[..47]) + } else if s.chars().count() > 50 { + let truncated: String = s.chars().take(47).collect(); + format!("{}...", truncated) } else { s.clone() } @@ -376,12 +378,15 @@ impl ForgeToolDisplay { } } -/// Truncate a string to max length +/// Truncate a string to max length, handling UTF-8 safely fn truncate_str(s: &str, max: usize) -> String { - if s.len() <= max { + let char_count = s.chars().count(); + if char_count <= max { s.to_string() } else { - format!("{}...", &s[..max.saturating_sub(3)]) + let truncate_to = max.saturating_sub(3); + let truncated: String = s.chars().take(truncate_to).collect(); + format!("{}...", truncated) } } diff --git a/src/analyzer/syncable-cli.code-workspace b/src/analyzer/syncable-cli.code-workspace new file mode 100644 index 00000000..407c7605 --- /dev/null +++ b/src/analyzer/syncable-cli.code-workspace @@ -0,0 +1,8 @@ +{ + "folders": [ + { + "path": "../.." + } + ], + "settings": {} +} \ No newline at end of file