fix(tui/sanitize): centralize diff/output sanitization; strip OSC/DCS/C1/zero-width and expand tabs via shared util\n\n- Add with and modes\n- Expand tabs (Unicode-aware) and remove zero-width/BiDi controls\n- Strip OSC/DCS/APC/PM/SOS strings and raw C0/C1 bytes; keep CSI when preserving ANSI\n- Apply to diff_render (plain) and history_cell (ANSI-preserving)\n\nValidated with ./build-fast.sh (no warnings).\n\nFixes #22

codex-rs/tui/src/diff_render.rs

@@ -10,12 +10,14 @@ use std::path::PathBuf;
 use codex_core::protocol::FileChange;
 
 use crate::history_cell::PatchEventType;
+use crate::sanitize::{sanitize_for_tui, Mode as SanitizeMode, Options as SanitizeOptions};
 
 // Sanitize diff content so tabs and control characters don’t break terminal layout.
 // Mirrors the behavior we use for user input and command output:
 // - Expand tabs to spaces using a fixed tab stop (4)
 // - Remove ASCII control characters (including ESC/CSI sequences) that could
 //   confuse terminal rendering; keep plain text only
+#[allow(dead_code)]
 fn expand_tabs_to_spaces(input: &str, tabstop: usize) -> String {
     let ts = tabstop.max(1);
     let mut out = String::with_capacity(input.len());
@@ -38,44 +40,137 @@ fn expand_tabs_to_spaces(input: &str, tabstop: usize) -> String {
     out
 }
 
+#[allow(dead_code)]
 fn strip_control_sequences(input: &str) -> String {
+    fn is_c1(ch: char) -> bool {
+        let u = ch as u32;
+        (0x80..=0x9F).contains(&u)
+    }
+
+    fn is_zero_width_or_bidi(ch: char) -> bool {
+        matches!(
+            ch,
+            // Zero-width, joiners, BOM
+            '\u{200B}' /* ZWSP */
+                | '\u{200C}' /* ZWNJ */
+                | '\u{200D}' /* ZWJ */
+                | '\u{2060}' /* WJ */
+                | '\u{FEFF}' /* BOM */
+                | '\u{00AD}' /* SOFT HYPHEN */
+                | '\u{180E}' /* MONGOLIAN VOWEL SEPARATOR (historic) */
+                // BiDi controls and isolates
+                | '\u{200E}' /* LRM */
+                | '\u{200F}' /* RLM */
+                | '\u{061C}' /* ALM */
+                | '\u{202A}' /* LRE */
+                | '\u{202B}' /* RLE */
+                | '\u{202D}' /* LRO */
+                | '\u{202E}' /* RLO */
+                | '\u{202C}' /* PDF */
+                | '\u{2066}' /* LRI */
+                | '\u{2067}' /* RLI */
+                | '\u{2068}' /* FSI */
+                | '\u{2069}' /* PDI */
+        )
+    }
+
+    fn consume_until_st_or_bel<I: Iterator<Item = char>>(it: &mut std::iter::Peekable<I>) {
+        while let Some(&c) = it.peek() {
+            match c {
+                // BEL terminator for OSC
+                '\u{0007}' => {
+                    it.next(); // eat BEL
+                    break;
+                }
+                // ST = ESC \
+                '\u{001B}' => {
+                    // lookahead for '\\'
+                    let _ = it.next(); // ESC
+                    if matches!(it.peek(), Some('\\')) {
+                        let _ = it.next(); // '\\'
+                        break;
+                    }
+                    // Otherwise, keep eating (this also drops nested sequences defensively)
+                }
+                _ => {
+                    let _ = it.next();
+                }
+            }
+        }
+    }
+
     let mut out = String::with_capacity(input.len());
     let mut chars = input.chars().peekable();
     while let Some(ch) = chars.next() {
-        if ch == '\u{001B}' {
-            // Skip a simple ESC [...] <alpha> CSI sequence, or generic ESC-seq until letter
-            if matches!(chars.peek(), Some('[')) {
-                // consume '['
-                let _ = chars.next();
-                // consume params until we hit an alphabetic final byte or end
-                while let Some(&c) = chars.peek() {
-                    if c.is_ascii_alphabetic() { chars.next(); break; }
-                    let _ = chars.next();
-                }
-            } else {
-                // Consume until an alphabetic; best‑effort strip of non‑CSI
-                while let Some(&c) = chars.peek() {
-                    if c.is_ascii_alphabetic() { let _ = chars.next(); break; }
-                    let _ = chars.next();
+        match ch {
+            // ESC-prefixed sequences
+            '\u{001B}' => {
+                match chars.peek().copied() {
+                    // CSI: ESC [ params/intermediates final
+                    Some('[') => {
+                        let _ = chars.next(); // '['
+                        // params 0x30..0x3F, intermediates 0x20..0x2F, final 0x40..0x7E
+                        while let Some(&c) = chars.peek() {
+                            let u = c as u32;
+                            if (0x40..=0x7E).contains(&u) {
+                                let _ = chars.next();
+                                break;
+                            } else {
+                                let _ = chars.next();
+                            }
+                        }
+                    }
+                    // OSC: ESC ] ... (BEL | ST)
+                    Some(']') => {
+                        let _ = chars.next(); // ']'
+                        consume_until_st_or_bel(&mut chars);
+                    }
+                    // String types (DCS/SOS/PM/APC): ESC P | X | ^ | _ ... ST
+                    Some('P') | Some('X') | Some('^') | Some('_') => {
+                        let _ = chars.next();
+                        consume_until_st_or_bel(&mut chars);
+                    }
+                    // Other ESC: consume optional intermediates then a final (0x40..0x7E)
+                    Some(_) | None => {
+                        // intermediates 0x20..0x2F
+                        while let Some(&c) = chars.peek() {
+                            let u = c as u32;
+                            if (0x20..=0x2F).contains(&u) {
+                                let _ = chars.next();
+                            } else {
+                                break;
+                            }
+                        }
+                        if let Some(&c) = chars.peek() {
+                            let u = c as u32;
+                            if (0x40..=0x7E).contains(&u) {
+                                let _ = chars.next();
+                            }
+                        }
+                    }
                 }
+                // In all ESC cases: skip emission
             }
-            continue;
+            // Drop other C0 control characters (0x00..0x1F, 0x7F)
+            c if (c as u32) < 0x20 || c == '\u{007F}' => {}
+            // Drop raw C1 controls if present (0x80..0x9F)
+            c if is_c1(c) => {}
+            // Drop zero-width and bidi controls that can affect layout
+            c if is_zero_width_or_bidi(c) => {}
+            // Keep printable character
+            _ => out.push(ch),
         }
-        // Drop other ASCII control characters (0x00..0x1F, 0x7F)
-        if (ch as u32) < 0x20 || ch == '\u{007F}' {
-            continue;
-        }
-        out.push(ch);
     }
     out
 }
 
 #[inline]
 fn sanitize_diff_text(s: &str) -> String {
-    // Order: first expand tabs (so control stripping doesn’t accidentally
-    // touch spaces we insert), then remove control sequences.
-    let expanded = expand_tabs_to_spaces(s, 4);
-    strip_control_sequences(&expanded)
+    sanitize_for_tui(
+        s,
+        SanitizeMode::Plain,
+        SanitizeOptions { expand_tabs: true, tabstop: 4 },
+    )
 }
 
 #[allow(dead_code)]

codex-rs/tui/src/history_cell.rs

@@ -37,7 +37,7 @@ use std::time::Instant;
 use std::time::SystemTime;
 use std::time::UNIX_EPOCH;
 use tracing::error;
-use unicode_width::UnicodeWidthChar;
+use crate::sanitize::{sanitize_for_tui, Mode as SanitizeMode, Options as SanitizeOptions};
 
 // ==================== Core Types ====================
 
@@ -4138,7 +4138,7 @@ fn normalize_overwrite_sequences(input: &str) -> String {
 fn build_preview_lines(text: &str, _include_left_pipe: bool) -> Vec<Line<'static>> {
     let processed = format_json_compact(text).unwrap_or_else(|| text.to_string());
     let processed = normalize_overwrite_sequences(&processed);
-    let processed = expand_tabs_to_spaces(&processed, 4);
+    let processed = sanitize_for_tui(&processed, SanitizeMode::AnsiPreserving, SanitizeOptions { expand_tabs: true, tabstop: 4 });
     let non_empty: Vec<&str> = processed.lines().filter(|line| !line.is_empty()).collect();
 
     enum Seg<'a> {
@@ -4207,7 +4207,7 @@ fn output_lines(
             format!("Error (exit code {})", exit_code),
             Style::default().fg(crate::colors::error()),
         ));
-        let stderr_norm = expand_tabs_to_spaces(&normalize_overwrite_sequences(stderr), 4);
+        let stderr_norm = sanitize_for_tui(&normalize_overwrite_sequences(stderr), SanitizeMode::AnsiPreserving, SanitizeOptions { expand_tabs: true, tabstop: 4 });
         for line in stderr_norm.lines().filter(|line| !line.is_empty()) {
             lines.push(ansi_escape_line(line).style(Style::default().fg(crate::colors::error())));
         }
@@ -4361,9 +4361,9 @@ pub(crate) fn new_user_prompt(message: String) -> PlainHistoryCell {
     // - Expand tabs to spaces with a fixed tab stop so wrapping is deterministic
     // - Parse ANSI sequences into spans so we never emit raw control bytes
     let normalized = normalize_overwrite_sequences(&message);
-    let expanded = expand_tabs_to_spaces(&normalized, 4);
+    let sanitized = sanitize_for_tui(&normalized, SanitizeMode::AnsiPreserving, SanitizeOptions { expand_tabs: true, tabstop: 4 });
     // Build content lines with ANSI converted to styled spans
-    let content: Vec<Line<'static>> = expanded.lines().map(|l| ansi_escape_line(l)).collect();
+    let content: Vec<Line<'static>> = sanitized.lines().map(|l| ansi_escape_line(l)).collect();
     let content = trim_empty_lines(content);
     lines.extend(content);
     // No empty line at end - trimming and spacing handled by renderer
@@ -4387,8 +4387,8 @@ pub(crate) fn new_queued_user_prompt(message: String) -> PlainHistoryCell {
     ]));
     // Normalize and render body like normal user messages
     let normalized = normalize_overwrite_sequences(&message);
-    let expanded = expand_tabs_to_spaces(&normalized, 4);
-    let content: Vec<Line<'static>> = expanded
+    let sanitized = sanitize_for_tui(&normalized, SanitizeMode::AnsiPreserving, SanitizeOptions { expand_tabs: true, tabstop: 4 });
+    let content: Vec<Line<'static>> = sanitized
         .lines()
         .map(|l| ansi_escape_line(l))
         .collect();
@@ -4401,34 +4401,7 @@ pub(crate) fn new_queued_user_prompt(message: String) -> PlainHistoryCell {
 /// This prevents terminals from applying their own tab expansion after
 /// ratatui has computed layout, which can otherwise cause glyphs to appear
 /// to "hang" or smear until overwritten.
-fn expand_tabs_to_spaces(input: &str, tabstop: usize) -> String {
-    let ts = tabstop.max(1);
-    let mut out = String::with_capacity(input.len());
-    for line in input.split_inclusive('\n') {
-        let mut col = 0usize; // display columns in this logical line
-        for ch in line.chars() {
-            match ch {
-                '\t' => {
-                    let spaces = ts - (col % ts);
-                    out.extend(std::iter::repeat(' ').take(spaces));
-                    col += spaces;
-                }
-                '\n' => {
-                    out.push('\n');
-                    col = 0;
-                }
-                _ => {
-                    out.push(ch);
-                    // Use Unicode width to advance columns for wide glyphs
-                    col += UnicodeWidthChar::width(ch).unwrap_or(1).max(1);
-                }
-            }
-        }
-        // If the line did not end with a newline, `split_inclusive` ensures we
-        // won't add one here (preserve exact trailing newline semantics).
-    }
-    out
-}
+// Tab expansion and control stripping are centralized in crate::sanitize
 
 #[allow(dead_code)]
 pub(crate) fn new_text_line(line: Line<'static>) -> PlainHistoryCell {
@@ -5377,7 +5350,7 @@ fn select_preview_from_lines(lines: &[Line<'static>], head: usize, tail: usize)
 fn select_preview_from_plain_text(text: &str, head: usize, tail: usize) -> Vec<Line<'static>> {
     let processed = format_json_compact(text).unwrap_or_else(|| text.to_string());
     let processed = normalize_overwrite_sequences(&processed);
-    let processed = expand_tabs_to_spaces(&processed, 4);
+    let processed = sanitize_for_tui(&processed, SanitizeMode::AnsiPreserving, SanitizeOptions { expand_tabs: true, tabstop: 4 });
     let non_empty: Vec<&str> = processed.lines().filter(|line| !line.is_empty()).collect();
     let mut out: Vec<Line<'static>> = Vec::new();
     if non_empty.len() <= head + tail {
@@ -6356,7 +6329,7 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell {
     ];
 
     let norm = normalize_overwrite_sequences(&stderr);
-    let norm = expand_tabs_to_spaces(&norm, 4);
+    let norm = sanitize_for_tui(&norm, SanitizeMode::AnsiPreserving, SanitizeOptions { expand_tabs: true, tabstop: 4 });
     for line in norm.lines() {
         if !line.is_empty() {
             lines.push(ansi_escape_line(line).fg(crate::colors::error()));

codex-rs/tui/src/lib.rs

@@ -57,6 +57,7 @@ mod shimmer;
 mod slash_command;
 mod resume;
 mod streaming;
+mod sanitize;
 mod terminal_info;
 // mod text_block; // Orphaned after trait-based HistoryCell migration
 mod text_formatting;

codex-rs/tui/src/sanitize.rs

@@ -0,0 +1,139 @@
+use unicode_width::UnicodeWidthChar;
+
+/// Sanitization mode:
+/// - Plain: remove all control/escape sequences; output is pure printable text.
+/// - AnsiPreserving: keep CSI (ESC '[' ... final) so color/style can be parsed
+///   later by the ANSI parser; strip OSC/DCS/APC/PM/SOS and other controls.
+#[derive(Clone, Copy)]
+pub enum Mode {
+    Plain,
+    AnsiPreserving,
+}
+
+#[derive(Clone, Copy)]
+pub struct Options {
+    pub expand_tabs: bool,
+    pub tabstop: usize,
+}
+
+impl Default for Options {
+    fn default() -> Self {
+        Options { expand_tabs: true, tabstop: 4 }
+    }
+}
+
+pub fn sanitize_for_tui(input: &str, mode: Mode, opts: Options) -> String {
+    // Optionally expand tabs first so that later stripping does not interact
+    // with spaces we insert.
+    let mut text = if opts.expand_tabs { expand_tabs_to_spaces(input, opts.tabstop) } else { input.to_string() };
+    text = strip_specials(text, mode);
+    text
+}
+
+fn expand_tabs_to_spaces(input: &str, tabstop: usize) -> String {
+    let ts = tabstop.max(1);
+    let mut out = String::with_capacity(input.len());
+    for line in input.split_inclusive('\n') {
+        let mut col = 0usize; // display columns in this logical line
+        for ch in line.chars() {
+            match ch {
+                '\t' => {
+                    let spaces = ts - (col % ts);
+                    out.extend(std::iter::repeat(' ').take(spaces));
+                    col += spaces;
+                }
+                '\n' => {
+                    out.push('\n');
+                    col = 0;
+                }
+                _ => {
+                    out.push(ch);
+                    // Advance columns using Unicode width so tabs align correctly
+                    col += UnicodeWidthChar::width(ch).unwrap_or(1).max(1);
+                }
+            }
+        }
+    }
+    out
+}
+
+fn strip_specials(input: String, mode: Mode) -> String {
+    // Work on chars to detect escape sequences and zero-width/bidi controls.
+    let mut out = String::with_capacity(input.len());
+    let mut it = input.chars().peekable();
+
+    // Helpers
+    fn is_c1(ch: char) -> bool { (0x80..=0x9F).contains(&(ch as u32)) }
+    fn is_zero_width_or_bidi(ch: char) -> bool {
+        matches!(
+            ch,
+            '\u{200B}' | '\u{200C}' | '\u{200D}' | '\u{2060}' | '\u{FEFF}' | '\u{00AD}' | '\u{180E}'
+                | '\u{200E}' | '\u{200F}' | '\u{061C}' | '\u{202A}' | '\u{202B}' | '\u{202D}'
+                | '\u{202E}' | '\u{202C}' | '\u{2066}' | '\u{2067}' | '\u{2068}' | '\u{2069}'
+        )
+    }
+    fn consume_until_st_or_bel<I: Iterator<Item = char>>(it: &mut std::iter::Peekable<I>) {
+        while let Some(&c) = it.peek() {
+            match c {
+                '\u{0007}' => { it.next(); break; } // BEL
+                '\u{001B}' => { // ESC
+                    it.next();
+                    if matches!(it.peek(), Some('\\')) { it.next(); break; } // ST = ESC \
+                }
+                _ => { it.next(); }
+            }
+        }
+    }
+
+    while let Some(ch) = it.next() {
+        match ch {
+            '\u{001B}' => {
+                match it.peek().copied() {
+                    // CSI: ESC [ ... final (0x40..0x7E)
+                    Some('[') => {
+                        // Keep if preserving ANSI; drop otherwise
+                        if let Mode::AnsiPreserving = mode {
+                            out.push('\u{001B}');
+                            out.push('[');
+                            it.next();
+                            while let Some(&c) = it.peek() {
+                                let u = c as u32;
+                                let is_final = (0x40..=0x7E).contains(&u);
+                                out.push(c);
+                                it.next();
+                                if is_final { break; }
+                            }
+                        } else {
+                            // Consume but do not emit
+                            it.next();
+                            while let Some(&c) = it.peek() {
+                                let u = c as u32;
+                                if (0x40..=0x7E).contains(&u) { it.next(); break; } else { it.next(); }
+                            }
+                        }
+                    }
+                    // OSC and other string types: strip in all modes
+                    Some(']') => { it.next(); consume_until_st_or_bel(&mut it); }
+                    Some('P') | Some('X') | Some('^') | Some('_') => { it.next(); consume_until_st_or_bel(&mut it); }
+                    // Other ESC sequences: drop
+                    Some(_) | None => {
+                        // intermediates 0x20..0x2F then a final 0x40..0x7E
+                        while let Some(&c) = it.peek() {
+                            let u = c as u32;
+                            if (0x20..=0x2F).contains(&u) { it.next(); } else { break; }
+                        }
+                        if let Some(&c) = it.peek() {
+                            let u = c as u32;
+                            if (0x40..=0x7E).contains(&u) { it.next(); }
+                        }
+                    }
+                }
+            }
+            c if (c as u32) < 0x20 || c == '\u{007F}' => { /* drop C0 */ }
+            c if is_c1(c) => { /* drop C1 */ }
+            c if is_zero_width_or_bidi(c) => { /* drop zero-width/bidi */ }
+            _ => out.push(ch),
+        }
+    }
+    out
+}