diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 67765a3e5..c4c9c85f1 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2318,6 +2318,7 @@ dependencies = [ "codex-tui", "codex-utils-cli", "crossterm", + "insta", "owo-colors", "pretty_assertions", "ratatui", diff --git a/codex-rs/cloud-tasks-client/Cargo.toml b/codex-rs/cloud-tasks-client/Cargo.toml index df8ec12b2..1a374d994 100644 --- a/codex-rs/cloud-tasks-client/Cargo.toml +++ b/codex-rs/cloud-tasks-client/Cargo.toml @@ -7,7 +7,7 @@ version.workspace = true [lib] name = "codex_cloud_tasks_client" path = "src/lib.rs" -test = false +test = true doctest = false [lints] diff --git a/codex-rs/cloud-tasks-client/src/http.rs b/codex-rs/cloud-tasks-client/src/http.rs index 46fed812b..cebcb60b7 100644 --- a/codex-rs/cloud-tasks-client/src/http.rs +++ b/codex-rs/cloud-tasks-client/src/http.rs @@ -862,7 +862,11 @@ mod api { if s.len() <= max { s.to_string() } else { - s[s.len() - max..].to_string() + let mut start = s.len() - max; + while !s.is_char_boundary(start) { + start += 1; + } + s[start..].to_string() } } @@ -885,7 +889,11 @@ mod api { .unwrap_or_else(|| "".to_string()); let head: String = patch.lines().take(20).collect::>().join("\n"); let head_trunc = if head.len() > 800 { - format!("{}…", &head[..800]) + let mut end = 800; + while !head.is_char_boundary(end) { + end -= 1; + } + format!("{}…", &head[..end]) } else { head }; @@ -893,6 +901,26 @@ mod api { "patch_summary: kind={kind} lines={lines} chars={chars} cwd={cwd} ; head=\n{head_trunc}" ) } + + #[cfg(test)] + mod tests { + use super::*; + + #[test] + fn tail_handles_a_multibyte_character_crossing_the_byte_limit() { + assert_eq!(tail("aé", /*max*/ 1), ""); + assert_eq!(tail("aé", /*max*/ 2), "é"); + } + + #[test] + fn patch_summary_handles_a_multibyte_character_crossing_the_byte_limit() { + let patch = format!("{}é", "a".repeat(799)); + + let summary = summarize_patch_for_logging(&patch); + + assert!(summary.ends_with(&format!("{}…", "a".repeat(799)))); + } + } } fn append_error_log(message: &str) { diff --git a/codex-rs/cloud-tasks/BUILD.bazel b/codex-rs/cloud-tasks/BUILD.bazel index 9beb5f87b..c55271d0a 100644 --- a/codex-rs/cloud-tasks/BUILD.bazel +++ b/codex-rs/cloud-tasks/BUILD.bazel @@ -4,4 +4,7 @@ codex_rust_crate( name = "cloud-tasks", crate_name = "codex_cloud_tasks", rustc_flags_extra = MACOS_WEBRTC_RUSTC_LINK_FLAGS, + test_data_extra = glob([ + "src/**/snapshots/**", + ]), ) diff --git a/codex-rs/cloud-tasks/Cargo.toml b/codex-rs/cloud-tasks/Cargo.toml index 7bdcaaddb..aa4245a7f 100644 --- a/codex-rs/cloud-tasks/Cargo.toml +++ b/codex-rs/cloud-tasks/Cargo.toml @@ -43,4 +43,5 @@ unicode-width = { workspace = true } workspace = true [dev-dependencies] +insta = { workspace = true } pretty_assertions = { workspace = true } diff --git a/codex-rs/cloud-tasks/src/lib.rs b/codex-rs/cloud-tasks/src/lib.rs index 080bbb211..252e7cca3 100644 --- a/codex-rs/cloud-tasks/src/lib.rs +++ b/codex-rs/cloud-tasks/src/lib.rs @@ -3,6 +3,7 @@ mod cli; pub(crate) mod env_detect; mod new_task; pub(crate) mod scrollable_diff; +mod truncation; mod ui; pub(crate) mod util; pub use cli::Cli; @@ -26,6 +27,7 @@ use supports_color::Stream as SupportStream; use tokio::sync::mpsc::UnboundedSender; use tracing::info; use tracing_subscriber::EnvFilter; +use truncation::truncate_to_byte_limit; use util::append_error_log; use util::format_relative_time; use util::set_user_agent_suffix; @@ -2108,7 +2110,7 @@ fn pretty_lines_from_error(raw: &str) -> Vec { if lines.len() == 1 { // Parsing yielded nothing; include a trimmed, short raw message tail for context. let tail = if raw.len() > 320 { - format!("{}…", &raw[..320]) + format!("{}…", truncate_to_byte_limit(raw, /*max_bytes*/ 320)) } else { raw.to_string() }; @@ -2142,6 +2144,19 @@ mod tests { use ratatui::buffer::Buffer; use ratatui::layout::Rect; + #[test] + fn error_fallback_handles_a_multibyte_character_crossing_the_byte_limit() { + let raw = format!("{}é", "a".repeat(319)); + + assert_eq!( + pretty_lines_from_error(&raw), + vec![ + "Failed to load task details.".to_string(), + format!("{}…", "a".repeat(319)), + ], + ); + } + struct StubGitInfo { default_branch: Option, current_branch: Option, diff --git a/codex-rs/cloud-tasks/src/snapshots/codex_cloud_tasks__ui__tests__footer_multibyte_status_truncation.snap b/codex-rs/cloud-tasks/src/snapshots/codex_cloud_tasks__ui__tests__footer_multibyte_status_truncation.snap new file mode 100644 index 000000000..03c1b2559 --- /dev/null +++ b/codex-rs/cloud-tasks/src/snapshots/codex_cloud_tasks__ui__tests__footer_multibyte_status_truncation.snap @@ -0,0 +1,8 @@ +--- +source: cloud-tasks/src/ui.rs +expression: terminal.backend() +--- +" " +"aaaa… " +" " +" " diff --git a/codex-rs/cloud-tasks/src/truncation.rs b/codex-rs/cloud-tasks/src/truncation.rs new file mode 100644 index 000000000..201b30b7a --- /dev/null +++ b/codex-rs/cloud-tasks/src/truncation.rs @@ -0,0 +1,18 @@ +pub(crate) fn truncate_to_byte_limit(value: &str, max_bytes: usize) -> &str { + let mut end = value.len().min(max_bytes); + while !value.is_char_boundary(end) { + end -= 1; + } + &value[..end] +} + +#[cfg(test)] +mod tests { + use super::truncate_to_byte_limit; + use pretty_assertions::assert_eq; + + #[test] + fn truncates_at_a_utf8_character_boundary() { + assert_eq!(truncate_to_byte_limit("aaaaéz", /*max_bytes*/ 5), "aaaa"); + } +} diff --git a/codex-rs/cloud-tasks/src/ui.rs b/codex-rs/cloud-tasks/src/ui.rs index 545c4455c..a805ca37e 100644 --- a/codex-rs/cloud-tasks/src/ui.rs +++ b/codex-rs/cloud-tasks/src/ui.rs @@ -20,12 +20,14 @@ use std::time::Instant; use crate::app::App; use crate::app::AttemptView; +use crate::truncation::truncate_to_byte_limit; use crate::util::format_relative_time_now; use codex_cloud_tasks_client::AttemptStatus; use codex_cloud_tasks_client::TaskStatus; use codex_tui::render_markdown_text; const CODEWITH_EMERALD: Color = Color::Green; +const STATUS_BYTE_LIMIT: usize = 2000; pub fn draw(frame: &mut Frame, app: &mut App) { let area = frame.area(); @@ -38,10 +40,10 @@ pub fn draw(frame: &mut Frame, app: &mut App) { .split(area); if app.new_task.is_some() { draw_new_task_page(frame, chunks[0], app); - draw_footer(frame, chunks[1], app); + draw_footer(frame, chunks[1], app, STATUS_BYTE_LIMIT); } else { draw_list(frame, chunks[0], app); - draw_footer(frame, chunks[1], app); + draw_footer(frame, chunks[1], app, STATUS_BYTE_LIMIT); } if app.diff_overlay.is_some() { @@ -235,7 +237,7 @@ fn draw_list(frame: &mut Frame, area: Rect, app: &mut App) { } } -fn draw_footer(frame: &mut Frame, area: Rect, app: &mut App) { +fn draw_footer(frame: &mut Frame, area: Rect, app: &mut App, status_byte_limit: usize) { let mut help = vec![ "↑/↓".dim(), ": Move ".dim(), @@ -300,10 +302,12 @@ fn draw_footer(frame: &mut Frame, area: Rect, app: &mut App) { // Bottom row: status/log text across full width (single-line; sanitize newlines) let mut status_line = app.status.replace('\n', " "); - if status_line.len() > 2000 { + if status_line.len() > status_byte_limit { // hard cap to avoid TUI noise - status_line.truncate(2000); - status_line.push('…'); + status_line = format!( + "{}…", + truncate_to_byte_limit(&status_line, status_byte_limit) + ); } // Clear the status row to avoid trailing characters when the message shrinks. frame.render_widget(Clear, rows[1]); @@ -468,6 +472,30 @@ fn draw_diff_overlay(frame: &mut Frame, area: Rect, app: &mut App) { } } +#[cfg(test)] +mod tests { + use super::*; + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + #[test] + fn footer_renders_a_multibyte_status_truncated_at_a_byte_boundary() { + let backend = TestBackend::new(12, 4); + let mut terminal = Terminal::new(backend).expect("create terminal"); + let mut app = App::new(); + app.status = "aaaaéz".to_string(); + + terminal + .draw(|frame| { + let area = frame.area(); + draw_footer(frame, area, &mut app, /*status_byte_limit*/ 5); + }) + .expect("draw footer"); + + insta::assert_snapshot!("footer_multibyte_status_truncation", terminal.backend()); + } +} + pub fn draw_apply_modal(frame: &mut Frame, area: Rect, app: &mut App) { use ratatui::widgets::Wrap; let inner = overlay_outer(area); diff --git a/codex-rs/rollout-trace/src/reducer/conversation/normalize.rs b/codex-rs/rollout-trace/src/reducer/conversation/normalize.rs index 92136a90f..79663b8d9 100644 --- a/codex-rs/rollout-trace/src/reducer/conversation/normalize.rs +++ b/codex-rs/rollout-trace/src/reducer/conversation/normalize.rs @@ -434,7 +434,11 @@ fn summarize_json(value: &Value) -> String { let mut summary = serde_json::to_string(value).unwrap_or_else(|_| "".to_string()); if summary.len() > MAX_JSON_SUMMARY_LEN { - summary.truncate(MAX_JSON_SUMMARY_LEN); + let mut end = MAX_JSON_SUMMARY_LEN; + while !summary.is_char_boundary(end) { + end -= 1; + } + summary.truncate(end); summary.push_str("..."); } summary @@ -446,3 +450,19 @@ fn u64_field(value: &Value, field: &str) -> Option { .and_then(Value::as_i64) .map(|value| value.max(0) as u64) } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use serde_json::json; + + #[test] + fn json_summary_handles_a_multibyte_character_crossing_the_byte_limit() { + let value = json!({"xy": "é".repeat(200)}); + + let summary = summarize_json(&value); + + assert_eq!(summary, format!("{{\"xy\":\"{}...", "é".repeat(116))); + } +}