From 0026b12615e546cbdb8d63dd85f303de33bc730c Mon Sep 17 00:00:00 2001 From: jif-oai Date: Thu, 9 Oct 2025 16:55:02 +0100 Subject: [PATCH 01/17] feat: indentation mode for read_file (#4887) Add a read file that select the region of the file based on the indentation level --- codex-rs/core/src/tools/handlers/read_file.rs | 928 ++++++++++++++++-- codex-rs/core/src/tools/spec.rs | 63 +- 2 files changed, 899 insertions(+), 92 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/read_file.rs b/codex-rs/core/src/tools/handlers/read_file.rs index 38b76f28ad..f9b6ae4dab 100644 --- a/codex-rs/core/src/tools/handlers/read_file.rs +++ b/codex-rs/core/src/tools/handlers/read_file.rs @@ -1,12 +1,9 @@ -use std::path::Path; +use std::collections::VecDeque; use std::path::PathBuf; use async_trait::async_trait; use codex_utils_string::take_bytes_at_char_boundary; use serde::Deserialize; -use tokio::fs::File; -use tokio::io::AsyncBufReadExt; -use tokio::io::BufReader; use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; @@ -18,22 +15,78 @@ use crate::tools::registry::ToolKind; pub struct ReadFileHandler; const MAX_LINE_LENGTH: usize = 500; +const TAB_WIDTH: usize = 4; -fn default_offset() -> usize { - 1 -} +// TODO(jif) add support for block comments +const COMMENT_PREFIXES: &[&str] = &["#", "//", "--"]; -fn default_limit() -> usize { - 2000 +/// JSON arguments accepted by the `read_file` tool handler. +#[derive(Deserialize)] +struct ReadFileArgs { + /// Absolute path to the file that will be read. + file_path: String, + /// 1-indexed line number to start reading from; defaults to 1. + #[serde(default = "defaults::offset")] + offset: usize, + /// Maximum number of lines to return; defaults to 2000. + #[serde(default = "defaults::limit")] + limit: usize, + /// Determines whether the handler reads a simple slice or indentation-aware block. + #[serde(default)] + mode: ReadMode, + /// Optional indentation configuration used when `mode` is `Indentation`. + #[serde(default)] + indentation: Option, } #[derive(Deserialize)] -struct ReadFileArgs { - file_path: String, - #[serde(default = "default_offset")] - offset: usize, - #[serde(default = "default_limit")] - limit: usize, +#[serde(rename_all = "snake_case")] +enum ReadMode { + Slice, + Indentation, +} +/// Additional configuration for indentation-aware reads. +#[derive(Deserialize, Clone)] +struct IndentationArgs { + /// Optional explicit anchor line; defaults to `offset` when omitted. + #[serde(default)] + anchor_line: Option, + /// Maximum indentation depth to collect; `0` means unlimited. + #[serde(default = "defaults::max_levels")] + max_levels: usize, + /// Whether to include sibling blocks at the same indentation level. + #[serde(default = "defaults::include_siblings")] + include_siblings: bool, + /// Whether to include header lines above the anchor block. This made on a best effort basis. + #[serde(default = "defaults::include_header")] + include_header: bool, + /// Optional hard cap on returned lines; defaults to the global `limit`. + #[serde(default)] + max_lines: Option, +} + +#[derive(Clone, Debug)] +struct LineRecord { + number: usize, + raw: String, + display: String, + indent: usize, +} + +impl LineRecord { + fn trimmed(&self) -> &str { + self.raw.trim_start() + } + + fn is_blank(&self) -> bool { + self.trimmed().is_empty() + } + + fn is_comment(&self) -> bool { + COMMENT_PREFIXES + .iter() + .any(|prefix| self.raw.trim().starts_with(prefix)) + } } #[async_trait] @@ -64,6 +117,8 @@ impl ToolHandler for ReadFileHandler { file_path, offset, limit, + mode, + indentation, } = args; if offset == 0 { @@ -85,7 +140,13 @@ impl ToolHandler for ReadFileHandler { )); } - let collected = read_file_slice(&path, offset, limit).await?; + let collected = match mode { + ReadMode::Slice => slice::read(&path, offset, limit).await?, + ReadMode::Indentation => { + let indentation = indentation.unwrap_or_default(); + indentation::read_block(&path, offset, limit, indentation).await? + } + }; Ok(ToolOutput::Function { content: collected.join("\n"), success: Some(true), @@ -93,62 +154,280 @@ impl ToolHandler for ReadFileHandler { } } -async fn read_file_slice( - path: &Path, - offset: usize, - limit: usize, -) -> Result, FunctionCallError> { - let file = File::open(path) - .await - .map_err(|err| FunctionCallError::RespondToModel(format!("failed to read file: {err}")))?; +mod slice { + use crate::function_tool::FunctionCallError; + use crate::tools::handlers::read_file::format_line; + use std::path::Path; + use tokio::fs::File; + use tokio::io::AsyncBufReadExt; + use tokio::io::BufReader; - let mut reader = BufReader::new(file); - let mut collected = Vec::new(); - let mut seen = 0usize; - let mut buffer = Vec::new(); - - loop { - buffer.clear(); - let bytes_read = reader.read_until(b'\n', &mut buffer).await.map_err(|err| { + pub async fn read( + path: &Path, + offset: usize, + limit: usize, + ) -> Result, FunctionCallError> { + let file = File::open(path).await.map_err(|err| { FunctionCallError::RespondToModel(format!("failed to read file: {err}")) })?; - if bytes_read == 0 { - break; - } + let mut reader = BufReader::new(file); + let mut collected = Vec::new(); + let mut seen = 0usize; + let mut buffer = Vec::new(); - if buffer.last() == Some(&b'\n') { - buffer.pop(); - if buffer.last() == Some(&b'\r') { + loop { + buffer.clear(); + let bytes_read = reader.read_until(b'\n', &mut buffer).await.map_err(|err| { + FunctionCallError::RespondToModel(format!("failed to read file: {err}")) + })?; + + if bytes_read == 0 { + break; + } + + if buffer.last() == Some(&b'\n') { buffer.pop(); + if buffer.last() == Some(&b'\r') { + buffer.pop(); + } + } + + seen += 1; + + if seen < offset { + continue; + } + + if collected.len() == limit { + break; + } + + let formatted = format_line(&buffer); + collected.push(format!("L{seen}: {formatted}")); + + if collected.len() == limit { + break; } } - seen += 1; - if seen < offset { - continue; + return Err(FunctionCallError::RespondToModel( + "offset exceeds file length".to_string(), + )); } - if collected.len() == limit { - break; + Ok(collected) + } +} + +mod indentation { + use crate::function_tool::FunctionCallError; + use crate::tools::handlers::read_file::IndentationArgs; + use crate::tools::handlers::read_file::LineRecord; + use crate::tools::handlers::read_file::TAB_WIDTH; + use crate::tools::handlers::read_file::format_line; + use crate::tools::handlers::read_file::trim_empty_lines; + use std::collections::VecDeque; + use std::path::Path; + use tokio::fs::File; + use tokio::io::AsyncBufReadExt; + use tokio::io::BufReader; + + pub async fn read_block( + path: &Path, + offset: usize, + limit: usize, + options: IndentationArgs, + ) -> Result, FunctionCallError> { + let anchor_line = options.anchor_line.unwrap_or(offset); + if anchor_line == 0 { + return Err(FunctionCallError::RespondToModel( + "anchor_line must be a 1-indexed line number".to_string(), + )); } - let formatted = format_line(&buffer); - collected.push(format!("L{seen}: {formatted}")); - - if collected.len() == limit { - break; + let guard_limit = options.max_lines.unwrap_or(limit); + if guard_limit == 0 { + return Err(FunctionCallError::RespondToModel( + "max_lines must be greater than zero".to_string(), + )); } + + let collected = collect_file_lines(path).await?; + if collected.is_empty() || anchor_line > collected.len() { + return Err(FunctionCallError::RespondToModel( + "anchor_line exceeds file length".to_string(), + )); + } + + let anchor_index = anchor_line - 1; + let effective_indents = compute_effective_indents(&collected); + let anchor_indent = effective_indents[anchor_index]; + + // Compute the min indent + let min_indent = if options.max_levels == 0 { + 0 + } else { + anchor_indent.saturating_sub(options.max_levels * TAB_WIDTH) + }; + + // Cap requested lines by guard_limit and file length + let final_limit = limit.min(guard_limit).min(collected.len()); + + if final_limit == 1 { + return Ok(vec![format!( + "L{}: {}", + collected[anchor_index].number, collected[anchor_index].display + )]); + } + + // Cursors + let mut i: isize = anchor_index as isize - 1; // up (inclusive) + let mut j: usize = anchor_index + 1; // down (inclusive) + let mut i_counter_min_indent = 0; + let mut j_counter_min_indent = 0; + + let mut out = VecDeque::with_capacity(limit); + out.push_back(&collected[anchor_index]); + + while out.len() < final_limit { + let mut progressed = 0; + + // Up. + if i >= 0 { + let iu = i as usize; + if effective_indents[iu] >= min_indent { + out.push_front(&collected[iu]); + progressed += 1; + i -= 1; + + // We do not include the siblings (not applied to comments). + if effective_indents[iu] == min_indent && !options.include_siblings { + let allow_header_comment = + options.include_header && collected[iu].is_comment(); + let can_take_line = allow_header_comment || i_counter_min_indent == 0; + + if can_take_line { + i_counter_min_indent += 1; + } else { + // This line shouldn't have been taken. + out.pop_front(); + progressed -= 1; + i = -1; // consider using Option or a control flag instead of a sentinel + } + } + + // Short-cut. + if out.len() >= final_limit { + break; + } + } else { + // Stop moving up. + i = -1; + } + } + + // Down. + if j < collected.len() { + let ju = j; + if effective_indents[ju] >= min_indent { + out.push_back(&collected[ju]); + progressed += 1; + j += 1; + + // We do not include the siblings (applied to comments). + if effective_indents[ju] == min_indent && !options.include_siblings { + if j_counter_min_indent > 0 { + // This line shouldn't have been taken. + out.pop_back(); + progressed -= 1; + j = collected.len(); + } + j_counter_min_indent += 1; + } + } else { + // Stop moving down. + j = collected.len(); + } + } + + if progressed == 0 { + break; + } + } + + // Trim empty lines + trim_empty_lines(&mut out); + + Ok(out + .into_iter() + .map(|record| format!("L{}: {}", record.number, record.display)) + .collect()) } - if seen < offset { - return Err(FunctionCallError::RespondToModel( - "offset exceeds file length".to_string(), - )); + async fn collect_file_lines(path: &Path) -> Result, FunctionCallError> { + let file = File::open(path).await.map_err(|err| { + FunctionCallError::RespondToModel(format!("failed to read file: {err}")) + })?; + + let mut reader = BufReader::new(file); + let mut buffer = Vec::new(); + let mut lines = Vec::new(); + let mut number = 0usize; + + loop { + buffer.clear(); + let bytes_read = reader.read_until(b'\n', &mut buffer).await.map_err(|err| { + FunctionCallError::RespondToModel(format!("failed to read file: {err}")) + })?; + + if bytes_read == 0 { + break; + } + + if buffer.last() == Some(&b'\n') { + buffer.pop(); + if buffer.last() == Some(&b'\r') { + buffer.pop(); + } + } + + number += 1; + let raw = String::from_utf8_lossy(&buffer).into_owned(); + let indent = measure_indent(&raw); + let display = format_line(&buffer); + lines.push(LineRecord { + number, + raw, + display, + indent, + }); + } + + Ok(lines) } - Ok(collected) + fn compute_effective_indents(records: &[LineRecord]) -> Vec { + let mut effective = Vec::with_capacity(records.len()); + let mut previous_indent = 0usize; + for record in records { + if record.is_blank() { + effective.push(previous_indent); + } else { + previous_indent = record.indent; + effective.push(previous_indent); + } + } + effective + } + + fn measure_indent(line: &str) -> usize { + line.chars() + .take_while(|c| matches!(c, ' ' | '\t')) + .map(|c| if c == '\t' { TAB_WIDTH } else { 1 }) + .sum() + } } fn format_line(bytes: &[u8]) -> String { @@ -160,93 +439,560 @@ fn format_line(bytes: &[u8]) -> String { } } +fn trim_empty_lines(out: &mut VecDeque<&LineRecord>) { + while matches!(out.front(), Some(line) if line.raw.trim().is_empty()) { + out.pop_front(); + } + while matches!(out.back(), Some(line) if line.raw.trim().is_empty()) { + out.pop_back(); + } +} + +mod defaults { + use super::*; + + impl Default for IndentationArgs { + fn default() -> Self { + Self { + anchor_line: None, + max_levels: max_levels(), + include_siblings: include_siblings(), + include_header: include_header(), + max_lines: None, + } + } + } + + impl Default for ReadMode { + fn default() -> Self { + Self::Slice + } + } + + pub fn offset() -> usize { + 1 + } + + pub fn limit() -> usize { + 2000 + } + + pub fn max_levels() -> usize { + 0 + } + + pub fn include_siblings() -> bool { + false + } + + pub fn include_header() -> bool { + true + } +} + #[cfg(test)] mod tests { + use super::indentation::read_block; + use super::slice::read; use super::*; + use pretty_assertions::assert_eq; use tempfile::NamedTempFile; #[tokio::test] - async fn reads_requested_range() { - let mut temp = NamedTempFile::new().expect("create temp file"); + async fn reads_requested_range() -> anyhow::Result<()> { + let mut temp = NamedTempFile::new()?; use std::io::Write as _; - writeln!(temp, "alpha").unwrap(); - writeln!(temp, "beta").unwrap(); - writeln!(temp, "gamma").unwrap(); + write!( + temp, + "alpha +beta +gamma +" + )?; - let lines = read_file_slice(temp.path(), 2, 2) - .await - .expect("read slice"); + let lines = read(temp.path(), 2, 2).await?; assert_eq!(lines, vec!["L2: beta".to_string(), "L3: gamma".to_string()]); + Ok(()) } #[tokio::test] - async fn errors_when_offset_exceeds_length() { - let mut temp = NamedTempFile::new().expect("create temp file"); + async fn errors_when_offset_exceeds_length() -> anyhow::Result<()> { + let mut temp = NamedTempFile::new()?; use std::io::Write as _; - writeln!(temp, "only").unwrap(); + writeln!(temp, "only")?; - let err = read_file_slice(temp.path(), 3, 1) + let err = read(temp.path(), 3, 1) .await .expect_err("offset exceeds length"); assert_eq!( err, FunctionCallError::RespondToModel("offset exceeds file length".to_string()) ); + Ok(()) } #[tokio::test] - async fn reads_non_utf8_lines() { - let mut temp = NamedTempFile::new().expect("create temp file"); + async fn reads_non_utf8_lines() -> anyhow::Result<()> { + let mut temp = NamedTempFile::new()?; use std::io::Write as _; - temp.as_file_mut().write_all(b"\xff\xfe\nplain\n").unwrap(); + temp.as_file_mut().write_all(b"\xff\xfe\nplain\n")?; - let lines = read_file_slice(temp.path(), 1, 2) - .await - .expect("read slice"); + let lines = read(temp.path(), 1, 2).await?; let expected_first = format!("L1: {}{}", '\u{FFFD}', '\u{FFFD}'); assert_eq!(lines, vec![expected_first, "L2: plain".to_string()]); + Ok(()) } #[tokio::test] - async fn trims_crlf_endings() { - let mut temp = NamedTempFile::new().expect("create temp file"); + async fn trims_crlf_endings() -> anyhow::Result<()> { + let mut temp = NamedTempFile::new()?; use std::io::Write as _; - write!(temp, "one\r\ntwo\r\n").unwrap(); + write!(temp, "one\r\ntwo\r\n")?; - let lines = read_file_slice(temp.path(), 1, 2) - .await - .expect("read slice"); + let lines = read(temp.path(), 1, 2).await?; assert_eq!(lines, vec!["L1: one".to_string(), "L2: two".to_string()]); + Ok(()) } #[tokio::test] - async fn respects_limit_even_with_more_lines() { - let mut temp = NamedTempFile::new().expect("create temp file"); + async fn respects_limit_even_with_more_lines() -> anyhow::Result<()> { + let mut temp = NamedTempFile::new()?; use std::io::Write as _; - writeln!(temp, "first").unwrap(); - writeln!(temp, "second").unwrap(); - writeln!(temp, "third").unwrap(); + write!( + temp, + "first +second +third +" + )?; - let lines = read_file_slice(temp.path(), 1, 2) - .await - .expect("read slice"); + let lines = read(temp.path(), 1, 2).await?; assert_eq!( lines, vec!["L1: first".to_string(), "L2: second".to_string()] ); + Ok(()) } #[tokio::test] - async fn truncates_lines_longer_than_max_length() { - let mut temp = NamedTempFile::new().expect("create temp file"); + async fn truncates_lines_longer_than_max_length() -> anyhow::Result<()> { + let mut temp = NamedTempFile::new()?; use std::io::Write as _; let long_line = "x".repeat(MAX_LINE_LENGTH + 50); - writeln!(temp, "{long_line}").unwrap(); + writeln!(temp, "{long_line}")?; - let lines = read_file_slice(temp.path(), 1, 1) - .await - .expect("read slice"); + let lines = read(temp.path(), 1, 1).await?; let expected = "x".repeat(MAX_LINE_LENGTH); assert_eq!(lines, vec![format!("L1: {expected}")]); + Ok(()) + } + + #[tokio::test] + async fn indentation_mode_captures_block() -> anyhow::Result<()> { + let mut temp = NamedTempFile::new()?; + use std::io::Write as _; + write!( + temp, + "fn outer() {{ + if cond {{ + inner(); + }} + tail(); +}} +" + )?; + + let options = IndentationArgs { + anchor_line: Some(3), + include_siblings: false, + max_levels: 1, + ..Default::default() + }; + + let lines = read_block(temp.path(), 3, 10, options).await?; + + assert_eq!( + lines, + vec![ + "L2: if cond {".to_string(), + "L3: inner();".to_string(), + "L4: }".to_string() + ] + ); + Ok(()) + } + + #[tokio::test] + async fn indentation_mode_expands_parents() -> anyhow::Result<()> { + let mut temp = NamedTempFile::new()?; + use std::io::Write as _; + write!( + temp, + "mod root {{ + fn outer() {{ + if cond {{ + inner(); + }} + }} +}} +" + )?; + + let mut options = IndentationArgs { + anchor_line: Some(4), + max_levels: 2, + ..Default::default() + }; + + let lines = read_block(temp.path(), 4, 50, options.clone()).await?; + assert_eq!( + lines, + vec![ + "L2: fn outer() {".to_string(), + "L3: if cond {".to_string(), + "L4: inner();".to_string(), + "L5: }".to_string(), + "L6: }".to_string(), + ] + ); + + options.max_levels = 3; + let expanded = read_block(temp.path(), 4, 50, options).await?; + assert_eq!( + expanded, + vec![ + "L1: mod root {".to_string(), + "L2: fn outer() {".to_string(), + "L3: if cond {".to_string(), + "L4: inner();".to_string(), + "L5: }".to_string(), + "L6: }".to_string(), + "L7: }".to_string(), + ] + ); + Ok(()) + } + + #[tokio::test] + async fn indentation_mode_respects_sibling_flag() -> anyhow::Result<()> { + let mut temp = NamedTempFile::new()?; + use std::io::Write as _; + write!( + temp, + "fn wrapper() {{ + if first {{ + do_first(); + }} + if second {{ + do_second(); + }} +}} +" + )?; + + let mut options = IndentationArgs { + anchor_line: Some(3), + include_siblings: false, + max_levels: 1, + ..Default::default() + }; + + let lines = read_block(temp.path(), 3, 50, options.clone()).await?; + assert_eq!( + lines, + vec![ + "L2: if first {".to_string(), + "L3: do_first();".to_string(), + "L4: }".to_string(), + ] + ); + + options.include_siblings = true; + let with_siblings = read_block(temp.path(), 3, 50, options).await?; + assert_eq!( + with_siblings, + vec![ + "L2: if first {".to_string(), + "L3: do_first();".to_string(), + "L4: }".to_string(), + "L5: if second {".to_string(), + "L6: do_second();".to_string(), + "L7: }".to_string(), + ] + ); + Ok(()) + } + + #[tokio::test] + async fn indentation_mode_handles_python_sample() -> anyhow::Result<()> { + let mut temp = NamedTempFile::new()?; + use std::io::Write as _; + write!( + temp, + "class Foo: + def __init__(self, size): + self.size = size + def double(self, value): + if value is None: + return 0 + result = value * self.size + return result +class Bar: + def compute(self): + helper = Foo(2) + return helper.double(5) +" + )?; + + let options = IndentationArgs { + anchor_line: Some(7), + include_siblings: true, + max_levels: 1, + ..Default::default() + }; + + let lines = read_block(temp.path(), 1, 200, options).await?; + assert_eq!( + lines, + vec![ + "L2: def __init__(self, size):".to_string(), + "L3: self.size = size".to_string(), + "L4: def double(self, value):".to_string(), + "L5: if value is None:".to_string(), + "L6: return 0".to_string(), + "L7: result = value * self.size".to_string(), + "L8: return result".to_string(), + ] + ); + Ok(()) + } + + #[tokio::test] + #[ignore] + async fn indentation_mode_handles_javascript_sample() -> anyhow::Result<()> { + let mut temp = NamedTempFile::new()?; + use std::io::Write as _; + write!( + temp, + "export function makeThing() {{ + const cache = new Map(); + function ensure(key) {{ + if (!cache.has(key)) {{ + cache.set(key, []); + }} + return cache.get(key); + }} + const handlers = {{ + init() {{ + console.log(\"init\"); + }}, + run() {{ + if (Math.random() > 0.5) {{ + return \"heads\"; + }} + return \"tails\"; + }}, + }}; + return {{ cache, handlers }}; +}} +export function other() {{ + return makeThing(); +}} +" + )?; + + let options = IndentationArgs { + anchor_line: Some(15), + max_levels: 1, + ..Default::default() + }; + + let lines = read_block(temp.path(), 15, 200, options).await?; + assert_eq!( + lines, + vec![ + "L10: init() {".to_string(), + "L11: console.log(\"init\");".to_string(), + "L12: },".to_string(), + "L13: run() {".to_string(), + "L14: if (Math.random() > 0.5) {".to_string(), + "L15: return \"heads\";".to_string(), + "L16: }".to_string(), + "L17: return \"tails\";".to_string(), + "L18: },".to_string(), + ] + ); + Ok(()) + } + + fn write_cpp_sample() -> anyhow::Result { + let mut temp = NamedTempFile::new()?; + use std::io::Write as _; + write!( + temp, + "#include +#include + +namespace sample {{ +class Runner {{ +public: + void setup() {{ + if (enabled_) {{ + init(); + }} + }} + + // Run the code + int run() const {{ + switch (mode_) {{ + case Mode::Fast: + return fast(); + case Mode::Slow: + return slow(); + default: + return fallback(); + }} + }} + +private: + bool enabled_ = false; + Mode mode_ = Mode::Fast; + + int fast() const {{ + return 1; + }} +}}; +}} // namespace sample +" + )?; + Ok(temp) + } + + #[tokio::test] + async fn indentation_mode_handles_cpp_sample_shallow() -> anyhow::Result<()> { + let temp = write_cpp_sample()?; + + let options = IndentationArgs { + include_siblings: false, + anchor_line: Some(18), + max_levels: 1, + ..Default::default() + }; + + let lines = read_block(temp.path(), 18, 200, options).await?; + assert_eq!( + lines, + vec![ + "L15: switch (mode_) {".to_string(), + "L16: case Mode::Fast:".to_string(), + "L17: return fast();".to_string(), + "L18: case Mode::Slow:".to_string(), + "L19: return slow();".to_string(), + "L20: default:".to_string(), + "L21: return fallback();".to_string(), + "L22: }".to_string(), + ] + ); + Ok(()) + } + + #[tokio::test] + async fn indentation_mode_handles_cpp_sample() -> anyhow::Result<()> { + let temp = write_cpp_sample()?; + + let options = IndentationArgs { + include_siblings: false, + anchor_line: Some(18), + max_levels: 2, + ..Default::default() + }; + + let lines = read_block(temp.path(), 18, 200, options).await?; + assert_eq!( + lines, + vec![ + "L13: // Run the code".to_string(), + "L14: int run() const {".to_string(), + "L15: switch (mode_) {".to_string(), + "L16: case Mode::Fast:".to_string(), + "L17: return fast();".to_string(), + "L18: case Mode::Slow:".to_string(), + "L19: return slow();".to_string(), + "L20: default:".to_string(), + "L21: return fallback();".to_string(), + "L22: }".to_string(), + "L23: }".to_string(), + ] + ); + Ok(()) + } + + #[tokio::test] + async fn indentation_mode_handles_cpp_sample_no_headers() -> anyhow::Result<()> { + let temp = write_cpp_sample()?; + + let options = IndentationArgs { + include_siblings: false, + include_header: false, + anchor_line: Some(18), + max_levels: 2, + ..Default::default() + }; + + let lines = read_block(temp.path(), 18, 200, options).await?; + assert_eq!( + lines, + vec![ + "L14: int run() const {".to_string(), + "L15: switch (mode_) {".to_string(), + "L16: case Mode::Fast:".to_string(), + "L17: return fast();".to_string(), + "L18: case Mode::Slow:".to_string(), + "L19: return slow();".to_string(), + "L20: default:".to_string(), + "L21: return fallback();".to_string(), + "L22: }".to_string(), + "L23: }".to_string(), + ] + ); + Ok(()) + } + + #[tokio::test] + async fn indentation_mode_handles_cpp_sample_siblings() -> anyhow::Result<()> { + let temp = write_cpp_sample()?; + + let options = IndentationArgs { + include_siblings: true, + include_header: false, + anchor_line: Some(18), + max_levels: 2, + ..Default::default() + }; + + let lines = read_block(temp.path(), 18, 200, options).await?; + assert_eq!( + lines, + vec![ + "L7: void setup() {".to_string(), + "L8: if (enabled_) {".to_string(), + "L9: init();".to_string(), + "L10: }".to_string(), + "L11: }".to_string(), + "L12: ".to_string(), + "L13: // Run the code".to_string(), + "L14: int run() const {".to_string(), + "L15: switch (mode_) {".to_string(), + "L16: case Mode::Fast:".to_string(), + "L17: return fast();".to_string(), + "L18: case Mode::Slow:".to_string(), + "L19: return slow();".to_string(), + "L20: default:".to_string(), + "L21: return fallback();".to_string(), + "L22: }".to_string(), + "L23: }".to_string(), + ] + ); + Ok(()) } } diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index e38095ecc9..c10f8e22f9 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -392,11 +392,72 @@ fn create_read_file_tool() -> ToolSpec { description: Some("The maximum number of lines to return.".to_string()), }, ); + properties.insert( + "mode".to_string(), + JsonSchema::String { + description: Some( + "Optional mode selector: \"slice\" for simple ranges (default) or \"indentation\" \ + to expand around an anchor line." + .to_string(), + ), + }, + ); + + let mut indentation_properties = BTreeMap::new(); + indentation_properties.insert( + "anchor_line".to_string(), + JsonSchema::Number { + description: Some( + "Anchor line to center the indentation lookup on (defaults to offset).".to_string(), + ), + }, + ); + indentation_properties.insert( + "max_levels".to_string(), + JsonSchema::Number { + description: Some( + "How many parent indentation levels (smaller indents) to include.".to_string(), + ), + }, + ); + indentation_properties.insert( + "include_siblings".to_string(), + JsonSchema::Boolean { + description: Some( + "When true, include additional blocks that share the anchor indentation." + .to_string(), + ), + }, + ); + indentation_properties.insert( + "include_header".to_string(), + JsonSchema::Boolean { + description: Some( + "Include doc comments or attributes directly above the selected block.".to_string(), + ), + }, + ); + indentation_properties.insert( + "max_lines".to_string(), + JsonSchema::Number { + description: Some( + "Hard cap on the number of lines returned when using indentation mode.".to_string(), + ), + }, + ); + properties.insert( + "indentation".to_string(), + JsonSchema::Object { + properties: indentation_properties, + required: None, + additional_properties: Some(false.into()), + }, + ); ToolSpec::Function(ResponsesApiTool { name: "read_file".to_string(), description: - "Reads a local file with 1-indexed line numbers and returns up to the requested number of lines." + "Reads a local file with 1-indexed line numbers, supporting slice and indentation-aware block modes." .to_string(), strict: false, parameters: JsonSchema::Object { From ca6a0358def87487a4a1186cae5a94591e7bd842 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Thu, 9 Oct 2025 17:01:01 +0100 Subject: [PATCH 02/17] bug: sandbox denied error logs (#4874) Check on STDOUT / STDERR or aggregated output for some logs when sanbox is denied --- codex-rs/core/src/error.rs | 108 ++++++++++++++++++++-- codex-rs/core/src/exec.rs | 128 +++++++++++++++++++++++++-- codex-rs/core/src/executor/runner.rs | 17 ++++ codex-rs/core/tests/suite/tools.rs | 73 --------------- 4 files changed, 237 insertions(+), 89 deletions(-) diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index 6fad448b44..786e20523a 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -1,6 +1,7 @@ use crate::exec::ExecToolCallOutput; use crate::token_data::KnownPlan; use crate::token_data::PlanType; +use crate::truncate::truncate_middle; use codex_protocol::ConversationId; use codex_protocol::protocol::RateLimitSnapshot; use reqwest::StatusCode; @@ -12,6 +13,9 @@ use tokio::task::JoinError; pub type Result = std::result::Result; +/// Limit UI error messages to a reasonable size while keeping useful context. +const ERROR_MESSAGE_UI_MAX_BYTES: usize = 2 * 1024; // 4 KiB + #[derive(Error, Debug)] pub enum SandboxErr { /// Error from sandbox execution @@ -304,21 +308,44 @@ impl CodexErr { } pub fn get_error_message_ui(e: &CodexErr) -> String { - match e { - CodexErr::Sandbox(SandboxErr::Denied { output }) => output.stderr.text.clone(), + let message = match e { + CodexErr::Sandbox(SandboxErr::Denied { output }) => { + let aggregated = output.aggregated_output.text.trim(); + if !aggregated.is_empty() { + output.aggregated_output.text.clone() + } else { + let stderr = output.stderr.text.trim(); + let stdout = output.stdout.text.trim(); + match (stderr.is_empty(), stdout.is_empty()) { + (false, false) => format!("{stderr}\n{stdout}"), + (false, true) => output.stderr.text.clone(), + (true, false) => output.stdout.text.clone(), + (true, true) => format!( + "command failed inside sandbox with exit code {}", + output.exit_code + ), + } + } + } // Timeouts are not sandbox errors from a UX perspective; present them plainly - CodexErr::Sandbox(SandboxErr::Timeout { output }) => format!( - "error: command timed out after {} ms", - output.duration.as_millis() - ), + CodexErr::Sandbox(SandboxErr::Timeout { output }) => { + format!( + "error: command timed out after {} ms", + output.duration.as_millis() + ) + } _ => e.to_string(), - } + }; + + truncate_middle(&message, ERROR_MESSAGE_UI_MAX_BYTES).0 } #[cfg(test)] mod tests { use super::*; + use crate::exec::StreamOutput; use codex_protocol::protocol::RateLimitWindow; + use pretty_assertions::assert_eq; fn rate_limit_snapshot() -> RateLimitSnapshot { RateLimitSnapshot { @@ -348,6 +375,73 @@ mod tests { ); } + #[test] + fn sandbox_denied_uses_aggregated_output_when_stderr_empty() { + let output = ExecToolCallOutput { + exit_code: 77, + stdout: StreamOutput::new(String::new()), + stderr: StreamOutput::new(String::new()), + aggregated_output: StreamOutput::new("aggregate detail".to_string()), + duration: Duration::from_millis(10), + timed_out: false, + }; + let err = CodexErr::Sandbox(SandboxErr::Denied { + output: Box::new(output), + }); + assert_eq!(get_error_message_ui(&err), "aggregate detail"); + } + + #[test] + fn sandbox_denied_reports_both_streams_when_available() { + let output = ExecToolCallOutput { + exit_code: 9, + stdout: StreamOutput::new("stdout detail".to_string()), + stderr: StreamOutput::new("stderr detail".to_string()), + aggregated_output: StreamOutput::new(String::new()), + duration: Duration::from_millis(10), + timed_out: false, + }; + let err = CodexErr::Sandbox(SandboxErr::Denied { + output: Box::new(output), + }); + assert_eq!(get_error_message_ui(&err), "stderr detail\nstdout detail"); + } + + #[test] + fn sandbox_denied_reports_stdout_when_no_stderr() { + let output = ExecToolCallOutput { + exit_code: 11, + stdout: StreamOutput::new("stdout only".to_string()), + stderr: StreamOutput::new(String::new()), + aggregated_output: StreamOutput::new(String::new()), + duration: Duration::from_millis(8), + timed_out: false, + }; + let err = CodexErr::Sandbox(SandboxErr::Denied { + output: Box::new(output), + }); + assert_eq!(get_error_message_ui(&err), "stdout only"); + } + + #[test] + fn sandbox_denied_reports_exit_code_when_no_output_available() { + let output = ExecToolCallOutput { + exit_code: 13, + stdout: StreamOutput::new(String::new()), + stderr: StreamOutput::new(String::new()), + aggregated_output: StreamOutput::new(String::new()), + duration: Duration::from_millis(5), + timed_out: false, + }; + let err = CodexErr::Sandbox(SandboxErr::Denied { + output: Box::new(output), + }); + assert_eq!( + get_error_message_ui(&err), + "command failed inside sandbox with exit code 13" + ); + } + #[test] fn usage_limit_reached_error_formats_free_plan() { let err = UsageLimitReachedError { diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index d84bbc9fcb..22d1deeb1c 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -177,7 +177,7 @@ pub async fn process_exec_tool_call( })); } - if exit_code != 0 && is_likely_sandbox_denied(sandbox_type, exit_code) { + if is_likely_sandbox_denied(sandbox_type, &exec_output) { return Err(CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(exec_output), })); @@ -195,21 +195,57 @@ pub async fn process_exec_tool_call( /// We don't have a fully deterministic way to tell if our command failed /// because of the sandbox - a command in the user's zshrc file might hit an /// error, but the command itself might fail or succeed for other reasons. -/// For now, we conservatively check for 'command not found' (exit code 127), -/// and can add additional cases as necessary. -fn is_likely_sandbox_denied(sandbox_type: SandboxType, exit_code: i32) -> bool { - if sandbox_type == SandboxType::None { +/// For now, we conservatively check for well known command failure exit codes and +/// also look for common sandbox denial keywords in the command output. +fn is_likely_sandbox_denied(sandbox_type: SandboxType, exec_output: &ExecToolCallOutput) -> bool { + if sandbox_type == SandboxType::None || exec_output.exit_code == 0 { return false; } // Quick rejects: well-known non-sandbox shell exit codes - // 127: command not found, 2: misuse of shell builtins - if exit_code == 127 { + // 2: misuse of shell builtins + // 126: permission denied + // 127: command not found + const QUICK_REJECT_EXIT_CODES: [i32; 3] = [2, 126, 127]; + if QUICK_REJECT_EXIT_CODES.contains(&exec_output.exit_code) { return false; } - // For all other cases, we assume the sandbox is the cause - true + const SANDBOX_DENIED_KEYWORDS: [&str; 6] = [ + "operation not permitted", + "permission denied", + "read-only file system", + "seccomp", + "sandbox", + "landlock", + ]; + + if [ + &exec_output.stderr.text, + &exec_output.stdout.text, + &exec_output.aggregated_output.text, + ] + .into_iter() + .any(|section| { + let lower = section.to_lowercase(); + SANDBOX_DENIED_KEYWORDS + .iter() + .any(|needle| lower.contains(needle)) + }) { + return true; + } + + #[cfg(unix)] + { + const SIGSYS_CODE: i32 = libc::SIGSYS; + if sandbox_type == SandboxType::LinuxSeccomp + && exec_output.exit_code == EXIT_CODE_SIGNAL_BASE + SIGSYS_CODE + { + return true; + } + } + + false } #[derive(Debug)] @@ -436,3 +472,77 @@ fn synthetic_exit_status(code: i32) -> ExitStatus { #[expect(clippy::unwrap_used)] std::process::ExitStatus::from_raw(code.try_into().unwrap()) } + +#[cfg(test)] +mod tests { + use super::*; + use std::time::Duration; + + fn make_exec_output( + exit_code: i32, + stdout: &str, + stderr: &str, + aggregated: &str, + ) -> ExecToolCallOutput { + ExecToolCallOutput { + exit_code, + stdout: StreamOutput::new(stdout.to_string()), + stderr: StreamOutput::new(stderr.to_string()), + aggregated_output: StreamOutput::new(aggregated.to_string()), + duration: Duration::from_millis(1), + timed_out: false, + } + } + + #[test] + fn sandbox_detection_requires_keywords() { + let output = make_exec_output(1, "", "", ""); + assert!(!is_likely_sandbox_denied( + SandboxType::LinuxSeccomp, + &output + )); + } + + #[test] + fn sandbox_detection_identifies_keyword_in_stderr() { + let output = make_exec_output(1, "", "Operation not permitted", ""); + assert!(is_likely_sandbox_denied(SandboxType::LinuxSeccomp, &output)); + } + + #[test] + fn sandbox_detection_respects_quick_reject_exit_codes() { + let output = make_exec_output(127, "", "command not found", ""); + assert!(!is_likely_sandbox_denied( + SandboxType::LinuxSeccomp, + &output + )); + } + + #[test] + fn sandbox_detection_ignores_non_sandbox_mode() { + let output = make_exec_output(1, "", "Operation not permitted", ""); + assert!(!is_likely_sandbox_denied(SandboxType::None, &output)); + } + + #[test] + fn sandbox_detection_uses_aggregated_output() { + let output = make_exec_output( + 101, + "", + "", + "cargo failed: Read-only file system when writing target", + ); + assert!(is_likely_sandbox_denied( + SandboxType::MacosSeatbelt, + &output + )); + } + + #[cfg(unix)] + #[test] + fn sandbox_detection_flags_sigsys_exit_code() { + let exit_code = EXIT_CODE_SIGNAL_BASE + libc::SIGSYS; + let output = make_exec_output(exit_code, "", "", ""); + assert!(is_likely_sandbox_denied(SandboxType::LinuxSeccomp, &output)); + } +} diff --git a/codex-rs/core/src/executor/runner.rs b/codex-rs/core/src/executor/runner.rs index f475aad67e..9a1956f66f 100644 --- a/codex-rs/core/src/executor/runner.rs +++ b/codex-rs/core/src/executor/runner.rs @@ -380,6 +380,23 @@ mod tests { assert_eq!(message, "failed in sandbox: sandbox stderr"); } + #[test] + fn sandbox_failure_message_falls_back_to_aggregated_output() { + let output = ExecToolCallOutput { + exit_code: 101, + stdout: StreamOutput::new(String::new()), + stderr: StreamOutput::new(String::new()), + aggregated_output: StreamOutput::new("aggregate text".to_string()), + duration: Duration::from_millis(10), + timed_out: false, + }; + let err = SandboxErr::Denied { + output: Box::new(output), + }; + let message = sandbox_failure_message(err); + assert_eq!(message, "failed in sandbox: aggregate text"); + } + #[test] fn normalize_function_error_synthesizes_payload() { let err = FunctionCallError::RespondToModel("boom".to_string()); diff --git a/codex-rs/core/tests/suite/tools.rs b/codex-rs/core/tests/suite/tools.rs index 99c70dbd61..ba87cb333d 100644 --- a/codex-rs/core/tests/suite/tools.rs +++ b/codex-rs/core/tests/suite/tools.rs @@ -403,79 +403,6 @@ async fn shell_timeout_includes_timeout_prefix_and_metadata() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn shell_sandbox_denied_truncates_error_output() -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let mut builder = test_codex(); - let test = builder.build(&server).await?; - - let call_id = "shell-denied"; - let long_line = "this is a long stderr line that should trigger truncation 0123456789abcdefghijklmnopqrstuvwxyz"; - let script = format!( - "for i in $(seq 1 500); do >&2 echo '{long_line}'; done; cat <<'EOF' > denied.txt\ncontent\nEOF", - ); - let args = json!({ - "command": ["/bin/sh", "-c", script], - "timeout_ms": 1_000, - }); - - mount_sse_once( - &server, - sse(vec![ - ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), - ev_completed("resp-1"), - ]), - ) - .await; - let second_mock = mount_sse_once( - &server, - sse(vec![ - ev_assistant_message("msg-1", "done"), - ev_completed("resp-2"), - ]), - ) - .await; - - submit_turn( - &test, - "attempt to write in read-only sandbox", - AskForApproval::Never, - SandboxPolicy::ReadOnly, - ) - .await?; - - let denied_item = second_mock.single_request().function_call_output(call_id); - - let output = denied_item - .get("output") - .and_then(Value::as_str) - .expect("denied output string"); - - let sandbox_pattern = r#"(?s)^Exit code: -?\d+ -Wall time: [0-9]+(?:\.[0-9]+)? seconds -Total output lines: \d+ -Output: - -failed in sandbox: .*?(?:Operation not permitted|Permission denied|Read-only file system).*? -\[\.{3} omitted \d+ of \d+ lines \.{3}\] -.*this is a long stderr line that should trigger truncation 0123456789abcdefghijklmnopqrstuvwxyz.* -\n?$"#; - let sandbox_regex = Regex::new(sandbox_pattern)?; - if !sandbox_regex.is_match(output) { - let fallback_pattern = r#"(?s)^Total output lines: \d+ - -failed in sandbox: this is a long stderr line that should trigger truncation 0123456789abcdefghijklmnopqrstuvwxyz -.*this is a long stderr line that should trigger truncation 0123456789abcdefghijklmnopqrstuvwxyz.* -.*(?:Operation not permitted|Permission denied|Read-only file system).*$"#; - assert_regex_match(fallback_pattern, output); - } - - Ok(()) -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn shell_spawn_failure_truncates_exec_error() -> Result<()> { skip_if_no_network!(Ok(())); From 3ddd4d47d07425da48ba5373d8bac14cca45f625 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Thu, 9 Oct 2025 17:06:07 +0100 Subject: [PATCH 03/17] fix: lagged output in unified_exec buffer (#4992) Handle `Lagged` error when the broadcast buffer of the unified_exec is full --- codex-rs/core/src/unified_exec/mod.rs | 21 +++- codex-rs/core/tests/suite/unified_exec.rs | 125 ++++++++++++++++++++++ 2 files changed, 141 insertions(+), 5 deletions(-) diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 48b82e3e8c..a8f754c92e 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -110,11 +110,22 @@ impl ManagedUnifiedExecSession { let buffer_clone = Arc::clone(&output_buffer); let notify_clone = Arc::clone(&output_notify); let output_task = tokio::spawn(async move { - while let Ok(chunk) = receiver.recv().await { - let mut guard = buffer_clone.lock().await; - guard.push_chunk(chunk); - drop(guard); - notify_clone.notify_waiters(); + loop { + match receiver.recv().await { + Ok(chunk) => { + let mut guard = buffer_clone.lock().await; + guard.push_chunk(chunk); + drop(guard); + notify_clone.notify_waiters(); + } + // If we lag behind the broadcast buffer, skip missed + // messages but keep the task alive to continue streaming. + Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => { + continue; + } + // When the sender closes, exit the task. + Err(tokio::sync::broadcast::error::RecvError::Closed) => break, + } } }); diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index 5afcc9f5fa..b8c0fadbbe 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -167,6 +167,131 @@ async fn unified_exec_reuses_session_via_stdin() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn unified_exec_streams_after_lagged_output() -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + + let server = start_mock_server().await; + + let mut builder = test_codex().with_config(|config| { + config.use_experimental_unified_exec_tool = true; + }); + let TestCodex { + codex, + cwd, + session_configured, + .. + } = builder.build(&server).await?; + + let script = r#"python3 - <<'PY' +import sys +import time + +chunk = b'x' * (1 << 20) +for _ in range(4): + sys.stdout.buffer.write(chunk) + sys.stdout.flush() + +time.sleep(0.2) +for _ in range(5): + sys.stdout.write("TAIL-MARKER\n") + sys.stdout.flush() + time.sleep(0.05) + +time.sleep(0.2) +PY +"#; + + let first_call_id = "uexec-lag-start"; + let first_args = serde_json::json!({ + "input": ["/bin/sh", "-c", script], + "timeout_ms": 25, + }); + + let second_call_id = "uexec-lag-poll"; + let second_args = serde_json::json!({ + "input": Vec::::new(), + "session_id": "0", + "timeout_ms": 800, + }); + + let responses = vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call( + first_call_id, + "unified_exec", + &serde_json::to_string(&first_args)?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_function_call( + second_call_id, + "unified_exec", + &serde_json::to_string(&second_args)?, + ), + ev_completed("resp-2"), + ]), + sse(vec![ + ev_assistant_message("msg-1", "lag handled"), + ev_completed("resp-3"), + ]), + ]; + mount_sse_sequence(&server, responses).await; + + let session_model = session_configured.model.clone(); + + codex + .submit(Op::UserTurn { + items: vec![InputItem::Text { + text: "exercise lag handling".into(), + }], + final_output_json_schema: None, + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: session_model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + wait_for_event(&codex, |event| matches!(event, EventMsg::TaskComplete(_))).await; + + let requests = server.received_requests().await.expect("recorded requests"); + assert!(!requests.is_empty(), "expected at least one POST request"); + + let bodies = requests + .iter() + .map(|req| req.body_json::().expect("request json")) + .collect::>(); + + let outputs = collect_tool_outputs(&bodies)?; + + let start_output = outputs + .get(first_call_id) + .expect("missing initial unified_exec output"); + let session_id = start_output["session_id"].as_str().unwrap_or_default(); + assert!( + !session_id.is_empty(), + "expected session id from initial unified_exec response" + ); + + let poll_output = outputs + .get(second_call_id) + .expect("missing poll unified_exec output"); + let poll_text = poll_output["output"].as_str().unwrap_or_default(); + assert!( + poll_text.contains("TAIL-MARKER"), + "expected poll output to contain tail marker, got {poll_text:?}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn unified_exec_timeout_and_followup_poll() -> Result<()> { skip_if_no_network!(Ok(())); From 7b4a4c2219e234b39266ffadb096fb0aad62bae9 Mon Sep 17 00:00:00 2001 From: Shijie Rao Date: Thu, 9 Oct 2025 09:42:24 -0700 Subject: [PATCH 04/17] Shijie/codesign binary (#4899) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Summary * Added code signing for MacOS. ### Before - UNSIGNED codex-aarch64 Screenshot 2025-10-08 at 11 53 28 AM ### After - SIGNED codex-aarch64 Screenshot 2025-10-08 at 11 52 20 AM --- .github/workflows/rust-release.yml | 134 ++++++++++++++++++++++++++++- 1 file changed, 132 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index aebdef5948..be04615699 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -47,7 +47,7 @@ jobs: build: needs: tag-check - name: ${{ matrix.runner }} - ${{ matrix.target }} + name: Build - ${{ matrix.runner }} - ${{ matrix.target }} runs-on: ${{ matrix.runner }} timeout-minutes: 30 defaults: @@ -94,11 +94,118 @@ jobs: - if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl'}} name: Install musl build tools run: | - sudo apt install -y musl-tools pkg-config + sudo apt-get update + sudo apt-get install -y musl-tools pkg-config - name: Cargo build run: cargo build --target ${{ matrix.target }} --release --bin codex --bin codex-responses-api-proxy + - if: ${{ matrix.runner == 'macos-14' }} + name: Configure Apple code signing + shell: bash + env: + KEYCHAIN_PASSWORD: actions + APPLE_CERTIFICATE: ${{ secrets.APPLE_CERTIFICATE_P12 }} + APPLE_CERTIFICATE_PASSWORD: ${{ secrets.APPLE_CERTIFICATE_PASSWORD }} + run: | + set -euo pipefail + + if [[ -z "${APPLE_CERTIFICATE:-}" ]]; then + echo "APPLE_CERTIFICATE is required for macOS signing" + exit 1 + fi + + if [[ -z "${APPLE_CERTIFICATE_PASSWORD:-}" ]]; then + echo "APPLE_CERTIFICATE_PASSWORD is required for macOS signing" + exit 1 + fi + + cert_path="${RUNNER_TEMP}/apple_signing_certificate.p12" + echo "$APPLE_CERTIFICATE" | base64 -d > "$cert_path" + + keychain_path="${RUNNER_TEMP}/codex-signing.keychain-db" + security create-keychain -p "$KEYCHAIN_PASSWORD" "$keychain_path" + security set-keychain-settings -lut 21600 "$keychain_path" + security unlock-keychain -p "$KEYCHAIN_PASSWORD" "$keychain_path" + + keychain_args=() + cleanup_keychain() { + if ((${#keychain_args[@]} > 0)); then + security list-keychains -s "${keychain_args[@]}" || true + security default-keychain -s "${keychain_args[0]}" || true + else + security list-keychains -s || true + fi + if [[ -f "$keychain_path" ]]; then + security delete-keychain "$keychain_path" || true + fi + } + + while IFS= read -r keychain; do + [[ -n "$keychain" ]] && keychain_args+=("$keychain") + done < <(security list-keychains | sed 's/^[[:space:]]*//;s/[[:space:]]*$//;s/"//g') + + if ((${#keychain_args[@]} > 0)); then + security list-keychains -s "$keychain_path" "${keychain_args[@]}" + else + security list-keychains -s "$keychain_path" + fi + + security default-keychain -s "$keychain_path" + security import "$cert_path" -k "$keychain_path" -P "$APPLE_CERTIFICATE_PASSWORD" -T /usr/bin/codesign -T /usr/bin/security + security set-key-partition-list -S apple-tool:,apple: -s -k "$KEYCHAIN_PASSWORD" "$keychain_path" > /dev/null + + codesign_hashes=() + while IFS= read -r hash; do + [[ -n "$hash" ]] && codesign_hashes+=("$hash") + done < <(security find-identity -v -p codesigning "$keychain_path" \ + | sed -n 's/.*\([0-9A-F]\{40\}\).*/\1/p' \ + | sort -u) + + if ((${#codesign_hashes[@]} == 0)); then + echo "No signing identities found in $keychain_path" + cleanup_keychain + rm -f "$cert_path" + exit 1 + fi + + if ((${#codesign_hashes[@]} > 1)); then + echo "Multiple signing identities found in $keychain_path:" + printf ' %s\n' "${codesign_hashes[@]}" + cleanup_keychain + rm -f "$cert_path" + exit 1 + fi + + APPLE_CODESIGN_IDENTITY="${codesign_hashes[0]}" + + rm -f "$cert_path" + + echo "APPLE_CODESIGN_IDENTITY=$APPLE_CODESIGN_IDENTITY" >> "$GITHUB_ENV" + echo "APPLE_CODESIGN_KEYCHAIN=$keychain_path" >> "$GITHUB_ENV" + echo "::add-mask::$APPLE_CODESIGN_IDENTITY" + + - if: ${{ matrix.runner == 'macos-14' }} + name: Sign macOS binaries + shell: bash + run: | + set -euo pipefail + + if [[ -z "${APPLE_CODESIGN_IDENTITY:-}" ]]; then + echo "APPLE_CODESIGN_IDENTITY is required for macOS signing" + exit 1 + fi + + keychain_args=() + if [[ -n "${APPLE_CODESIGN_KEYCHAIN:-}" && -f "${APPLE_CODESIGN_KEYCHAIN}" ]]; then + keychain_args+=(--keychain "${APPLE_CODESIGN_KEYCHAIN}") + fi + + for binary in codex codex-responses-api-proxy; do + path="target/${{ matrix.target }}/release/${binary}" + codesign --force --options runtime --timestamp --sign "$APPLE_CODESIGN_IDENTITY" "${keychain_args[@]}" "$path" + done + - name: Stage artifacts shell: bash run: | @@ -157,6 +264,29 @@ jobs: zstd -T0 -19 --rm "$dest/$base" done + - name: Remove signing keychain + if: ${{ always() && matrix.runner == 'macos-14' }} + shell: bash + env: + APPLE_CODESIGN_KEYCHAIN: ${{ env.APPLE_CODESIGN_KEYCHAIN }} + run: | + set -euo pipefail + if [[ -n "${APPLE_CODESIGN_KEYCHAIN:-}" ]]; then + keychain_args=() + while IFS= read -r keychain; do + [[ "$keychain" == "$APPLE_CODESIGN_KEYCHAIN" ]] && continue + [[ -n "$keychain" ]] && keychain_args+=("$keychain") + done < <(security list-keychains | sed 's/^[[:space:]]*//;s/[[:space:]]*$//;s/"//g') + if ((${#keychain_args[@]} > 0)); then + security list-keychains -s "${keychain_args[@]}" + security default-keychain -s "${keychain_args[0]}" + fi + + if [[ -f "$APPLE_CODESIGN_KEYCHAIN" ]]; then + security delete-keychain "$APPLE_CODESIGN_KEYCHAIN" + fi + fi + - uses: actions/upload-artifact@v4 with: name: ${{ matrix.target }} From 0308febc2324a3759215dbf92fb61c81db608243 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 9 Oct 2025 10:29:22 -0700 Subject: [PATCH 05/17] Remove unused type (#5003) It was never exported --- sdk/typescript/src/items.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/sdk/typescript/src/items.ts b/sdk/typescript/src/items.ts index 37563f9af9..fa2e0326bf 100644 --- a/sdk/typescript/src/items.ts +++ b/sdk/typescript/src/items.ts @@ -102,11 +102,6 @@ export type TodoListItem = { items: TodoItem[]; }; -export type SessionItem = { - id: string; - type: "session"; - session_id: string; -}; /** Canonical union of thread items and their type-specific payloads. */ export type ThreadItem = From bf82353f451dd71a1a9696fc595444fca04cc6d6 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Thu, 9 Oct 2025 10:37:13 -0700 Subject: [PATCH 06/17] tui: fix wrapping in user approval decisions (#5008) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit before: Screenshot 2025-10-09 at 10 20 57 AM after: Screenshot 2025-10-09 at 10 20 35 AM --- .../tui/src/bottom_pane/approval_overlay.rs | 118 ++++---------- ...pproval_history_decision_aborted_long.snap | 4 +- codex-rs/tui/src/history_cell.rs | 150 +++++++++++++++++- codex-rs/tui/src/pager_overlay.rs | 8 +- 4 files changed, 184 insertions(+), 96 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 6585124c52..d6e96cf78d 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -16,7 +16,6 @@ use crate::key_hint::KeyBinding; use crate::render::highlight::highlight_bash_to_lines; use crate::render::renderable::ColumnRenderable; use crate::render::renderable::Renderable; -use crate::text_formatting::truncate_text; use codex_core::protocol::FileChange; use codex_core::protocol::Op; use codex_core::protocol::ReviewDecision; @@ -160,11 +159,8 @@ impl ApprovalOverlay { } fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) { - if let Some(lines) = build_exec_history_lines(command.to_vec(), decision) { - self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( - history_cell::new_user_approval_decision(lines), - ))); - } + let cell = history_cell::new_approval_decision_cell(command.to_vec(), decision); + self.app_event_tx.send(AppEvent::InsertHistoryCell(cell)); self.app_event_tx.send(AppEvent::CodexOp(Op::ExecApproval { id: id.to_string(), decision, @@ -396,91 +392,11 @@ fn patch_options() -> Vec { ] } -fn build_exec_history_lines( - command: Vec, - decision: ReviewDecision, -) -> Option>> { - use ReviewDecision::*; - - let (symbol, summary): (Span<'static>, Vec>) = match decision { - Approved => { - let snippet = Span::from(exec_snippet(&command)).dim(); - ( - "✔ ".green(), - vec![ - "You ".into(), - "approved".bold(), - " codex to run ".into(), - snippet, - " this time".bold(), - ], - ) - } - ApprovedForSession => { - let snippet = Span::from(exec_snippet(&command)).dim(); - ( - "✔ ".green(), - vec![ - "You ".into(), - "approved".bold(), - " codex to run ".into(), - snippet, - " every time this session".bold(), - ], - ) - } - Denied => { - let snippet = Span::from(exec_snippet(&command)).dim(); - ( - "✗ ".red(), - vec![ - "You ".into(), - "did not approve".bold(), - " codex to run ".into(), - snippet, - ], - ) - } - Abort => { - let snippet = Span::from(exec_snippet(&command)).dim(); - ( - "✗ ".red(), - vec![ - "You ".into(), - "canceled".bold(), - " the request to run ".into(), - snippet, - ], - ) - } - }; - - let mut lines = Vec::new(); - let mut spans = Vec::new(); - spans.push(symbol); - spans.extend(summary); - lines.push(Line::from(spans)); - Some(lines) -} - -fn truncate_exec_snippet(full_cmd: &str) -> String { - let mut snippet = match full_cmd.split_once('\n') { - Some((first, _)) => format!("{first} ..."), - None => full_cmd.to_string(), - }; - snippet = truncate_text(&snippet, 80); - snippet -} - -fn exec_snippet(command: &[String]) -> String { - let full_cmd = strip_bash_lc_and_escape(command); - truncate_exec_snippet(&full_cmd) -} - #[cfg(test)] mod tests { use super::*; use crate::app_event::AppEvent; + use pretty_assertions::assert_eq; use tokio::sync::mpsc::unbounded_channel; fn make_exec_request() -> ApprovalRequest { @@ -550,6 +466,34 @@ mod tests { ); } + #[test] + fn exec_history_cell_wraps_with_two_space_indent() { + let command = vec![ + "/bin/zsh".into(), + "-lc".into(), + "git add tui/src/render/mod.rs tui/src/render/renderable.rs".into(), + ]; + let cell = history_cell::new_approval_decision_cell(command, ReviewDecision::Approved); + let lines = cell.display_lines(28); + let rendered: Vec = lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect(); + let expected = vec![ + "✔ You approved codex to".to_string(), + " run /bin/zsh -lc 'git add".to_string(), + " tui/src/render/mod.rs tui/".to_string(), + " src/render/renderable.rs'".to_string(), + " this time".to_string(), + ]; + assert_eq!(rendered, expected); + } + #[test] fn enter_sets_last_selected_index_without_dismissing() { let (tx_raw, mut rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_history_decision_aborted_long.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_history_decision_aborted_long.snap index 524e096824..f04e1f078a 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_history_decision_aborted_long.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_history_decision_aborted_long.snap @@ -1,5 +1,7 @@ --- source: tui/src/chatwidget/tests.rs +assertion_line: 495 expression: lines_to_single_string(&aborted_long) --- -✗ You canceled the request to run echo aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa... +✗ You canceled the request to run echo + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa... diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 151712b6b0..18a6c72d24 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -6,12 +6,15 @@ use crate::exec_cell::TOOL_CALL_MAX_LINES; use crate::exec_cell::output_lines; use crate::exec_cell::spinner; use crate::exec_command::relativize_to_home; +use crate::exec_command::strip_bash_lc_and_escape; use crate::markdown::MarkdownCitationContext; use crate::markdown::append_markdown; use crate::render::line_utils::line_to_static; use crate::render::line_utils::prefix_lines; +use crate::render::line_utils::push_owned_lines; use crate::style::user_message_style; use crate::text_formatting::format_and_truncate_tool_result; +use crate::text_formatting::truncate_text; use crate::ui_consts::LIVE_PREFIX_COLS; use crate::wrapping::RtOptions; use crate::wrapping::word_wrap_line; @@ -262,6 +265,126 @@ impl HistoryCell for PlainHistoryCell { } } +#[derive(Debug)] +pub(crate) struct PrefixedWrappedHistoryCell { + text: Text<'static>, + initial_prefix: Line<'static>, + subsequent_prefix: Line<'static>, +} + +impl PrefixedWrappedHistoryCell { + pub(crate) fn new( + text: impl Into>, + initial_prefix: impl Into>, + subsequent_prefix: impl Into>, + ) -> Self { + Self { + text: text.into(), + initial_prefix: initial_prefix.into(), + subsequent_prefix: subsequent_prefix.into(), + } + } +} + +impl HistoryCell for PrefixedWrappedHistoryCell { + fn display_lines(&self, width: u16) -> Vec> { + if width == 0 { + return Vec::new(); + } + let opts = RtOptions::new(width.max(1) as usize) + .initial_indent(self.initial_prefix.clone()) + .subsequent_indent(self.subsequent_prefix.clone()); + let wrapped = word_wrap_lines(&self.text, opts); + let mut out = Vec::new(); + push_owned_lines(&wrapped, &mut out); + out + } + + fn desired_height(&self, width: u16) -> u16 { + self.display_lines(width).len() as u16 + } +} + +fn truncate_exec_snippet(full_cmd: &str) -> String { + let mut snippet = match full_cmd.split_once('\n') { + Some((first, _)) => format!("{first} ..."), + None => full_cmd.to_string(), + }; + snippet = truncate_text(&snippet, 80); + snippet +} + +fn exec_snippet(command: &[String]) -> String { + let full_cmd = strip_bash_lc_and_escape(command); + truncate_exec_snippet(&full_cmd) +} + +pub fn new_approval_decision_cell( + command: Vec, + decision: codex_core::protocol::ReviewDecision, +) -> Box { + use codex_core::protocol::ReviewDecision::*; + + let (symbol, summary): (Span<'static>, Vec>) = match decision { + Approved => { + let snippet = Span::from(exec_snippet(&command)).dim(); + ( + "✔ ".green(), + vec![ + "You ".into(), + "approved".bold(), + " codex to run ".into(), + snippet, + " this time".bold(), + ], + ) + } + ApprovedForSession => { + let snippet = Span::from(exec_snippet(&command)).dim(); + ( + "✔ ".green(), + vec![ + "You ".into(), + "approved".bold(), + " codex to run ".into(), + snippet, + " every time this session".bold(), + ], + ) + } + Denied => { + let snippet = Span::from(exec_snippet(&command)).dim(); + ( + "✗ ".red(), + vec![ + "You ".into(), + "did not approve".bold(), + " codex to run ".into(), + snippet, + ], + ) + } + Abort => { + let snippet = Span::from(exec_snippet(&command)).dim(); + ( + "✗ ".red(), + vec![ + "You ".into(), + "canceled".bold(), + " the request to run ".into(), + snippet, + ], + ) + } + }; + + Box::new(PrefixedWrappedHistoryCell::new( + Line::from(summary), + symbol, + " ", + )) +} + /// Cyan history cell line showing the current review status. pub(crate) fn new_review_status_line(message: String) -> PlainHistoryCell { PlainHistoryCell { @@ -447,10 +570,6 @@ pub(crate) fn new_user_prompt(message: String) -> UserHistoryCell { UserHistoryCell { message } } -pub(crate) fn new_user_approval_decision(lines: Vec>) -> PlainHistoryCell { - PlainHistoryCell { lines } -} - #[derive(Debug)] struct SessionHeaderHistoryCell { version: &'static str, @@ -1203,6 +1322,29 @@ mod tests { assert_eq!(cell.desired_transcript_height(80), 1); } + #[test] + fn prefixed_wrapped_history_cell_indents_wrapped_lines() { + let summary = Line::from(vec![ + "You ".into(), + "approved".bold(), + " codex to run ".into(), + "echo something really long to ensure wrapping happens".dim(), + " this time".bold(), + ]); + let cell = PrefixedWrappedHistoryCell::new(summary, "✔ ".green(), " "); + let rendered = render_lines(&cell.display_lines(24)); + assert_eq!( + rendered, + vec![ + "✔ You approved codex".to_string(), + " to run echo something".to_string(), + " really long to ensure".to_string(), + " wrapping happens this".to_string(), + " time".to_string(), + ] + ); + } + #[test] fn active_mcp_tool_call_snapshot() { let invocation = McpInvocation { diff --git a/codex-rs/tui/src/pager_overlay.rs b/codex-rs/tui/src/pager_overlay.rs index 46c6c19a18..9395fb71dd 100644 --- a/codex-rs/tui/src/pager_overlay.rs +++ b/codex-rs/tui/src/pager_overlay.rs @@ -582,6 +582,7 @@ fn render_offset_content( #[cfg(test)] mod tests { use super::*; + use codex_core::protocol::ReviewDecision; use insta::assert_snapshot; use std::collections::HashMap; use std::path::PathBuf; @@ -589,6 +590,7 @@ mod tests { use std::time::Duration; use crate::exec_cell::CommandOutput; + use crate::history_cell; use crate::history_cell::HistoryCell; use crate::history_cell::new_patch_event; use codex_core::protocol::FileChange; @@ -712,10 +714,8 @@ mod tests { cells.push(apply_begin_cell); let apply_end_cell: Arc = - Arc::new(crate::history_cell::new_user_approval_decision(vec![ - "✓ Patch applied".green().bold().into(), - "src/foo.txt".dim().into(), - ])); + history_cell::new_approval_decision_cell(vec!["ls".into()], ReviewDecision::Approved) + .into(); cells.push(apply_end_cell); let mut exec_cell = crate::exec_cell::new_active_exec_command( From 95b41dd7f18b85a696758e3efe242c8880e9755a Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Thu, 9 Oct 2025 10:39:45 -0700 Subject: [PATCH 07/17] tui: fix wrapping in trust_directory (#5007) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor trust_directory to use ColumnRenderable & friends, thus correcting wrapping behavior at small widths. Also introduce RowRenderable with fixed-width rows. - fixed wrapping in trust_directory - changed selector cursor to match other list item selections - allow y/n to work as well as 1/2 - fixed key_hint to be standard before: Screenshot 2025-10-09 at 9 50 36 AM after: Screenshot 2025-10-09 at 9 51 31 AM --- .../tui/src/bottom_pane/approval_overlay.rs | 8 +- .../src/bottom_pane/list_selection_view.rs | 2 +- ...exec_and_status_layout_vt100_snapshot.snap | 1 + codex-rs/tui/src/diff_render.rs | 2 +- ..._tests__renders_snapshot_for_git_repo.snap | 14 ++ .../tui/src/onboarding/trust_directory.rs | 171 ++++++++++++------ codex-rs/tui/src/render/mod.rs | 8 +- codex-rs/tui/src/render/renderable.rs | 78 +++++++- codex-rs/tui/src/test_backend.rs | 3 +- codex-rs/tui/src/wrapping.rs | 1 + 10 files changed, 220 insertions(+), 68 deletions(-) create mode 100644 codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index d6e96cf78d..ce0e3c9c95 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -104,9 +104,9 @@ impl ApprovalOverlay { ), }; - let header = Box::new(ColumnRenderable::new([ - Box::new(Line::from(title.bold())), - Box::new(Line::from("")), + let header = Box::new(ColumnRenderable::with([ + Line::from(title.bold()).into(), + Line::from("").into(), header, ])); @@ -323,7 +323,7 @@ impl From for ApprovalRequestState { header.push(DiffSummary::new(changes, cwd).into()); Self { variant: ApprovalVariant::ApplyPatch { id }, - header: Box::new(ColumnRenderable::new(header)), + header: Box::new(ColumnRenderable::with(header)), } } } diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index 238f74d6e4..1e8d1983b0 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -87,7 +87,7 @@ impl ListSelectionView { if params.title.is_some() || params.subtitle.is_some() { let title = params.title.map(|title| Line::from(title.bold())); let subtitle = params.subtitle.map(|subtitle| Line::from(subtitle.dim())); - header = Box::new(ColumnRenderable::new([ + header = Box::new(ColumnRenderable::with([ header, Box::new(title), Box::new(subtitle), diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap index f913ce0898..c3bdf60bd2 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap @@ -13,4 +13,5 @@ expression: term.backend().vt100().screen().contents() › Summarize recent commits + 100% context left diff --git a/codex-rs/tui/src/diff_render.rs b/codex-rs/tui/src/diff_render.rs index a4b7c87f73..665fad15f7 100644 --- a/codex-rs/tui/src/diff_render.rs +++ b/codex-rs/tui/src/diff_render.rs @@ -72,7 +72,7 @@ impl From for Box { ))); } - Box::new(ColumnRenderable::new(rows)) + Box::new(ColumnRenderable::with(rows)) } } diff --git a/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap b/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap new file mode 100644 index 0000000000..71cbc3d6ea --- /dev/null +++ b/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/onboarding/trust_directory.rs +expression: terminal.backend() +--- +> You are running Codex in /workspace/project + + Since this folder is version controlled, you may wish to allow Codex + to work in this folder without asking for approval. + +› 1. Yes, allow Codex to work in this folder without asking for + approval + 2. No, ask me to approve edits and commands + + Press enter to continue diff --git a/codex-rs/tui/src/onboarding/trust_directory.rs b/codex-rs/tui/src/onboarding/trust_directory.rs index 1058208bee..3324c3d866 100644 --- a/codex-rs/tui/src/onboarding/trust_directory.rs +++ b/codex-rs/tui/src/onboarding/trust_directory.rs @@ -7,19 +7,25 @@ use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; use ratatui::buffer::Buffer; use ratatui::layout::Rect; -use ratatui::prelude::Widget; -use ratatui::style::Color; -use ratatui::style::Modifier; +use ratatui::style::Style; +use ratatui::style::Styled as _; use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; +use crate::key_hint; use crate::onboarding::onboarding_screen::KeyboardHandler; use crate::onboarding::onboarding_screen::StepStateProvider; +use crate::render::Insets; +use crate::render::renderable::ColumnRenderable; +use crate::render::renderable::Renderable; +use crate::render::renderable::RenderableExt as _; +use crate::render::renderable::RowRenderable; use super::onboarding_screen::StepState; +use unicode_width::UnicodeWidthStr; pub(crate) struct TrustDirectoryWidget { pub codex_home: PathBuf, @@ -38,76 +44,106 @@ pub enum TrustDirectorySelection { impl WidgetRef for &TrustDirectoryWidget { fn render_ref(&self, area: Rect, buf: &mut Buffer) { - let mut lines: Vec = vec![ - Line::from(vec![ - "> ".into(), - "You are running Codex in ".bold(), - self.cwd.to_string_lossy().to_string().into(), - ]), - "".into(), - ]; + let mut column = ColumnRenderable::new(); - if self.is_git_repo { - lines.push( - " Since this folder is version controlled, you may wish to allow Codex".into(), - ); - lines.push(" to work in this folder without asking for approval.".into()); + column.push(Line::from(vec![ + "> ".into(), + "You are running Codex in ".bold(), + self.cwd.to_string_lossy().to_string().into(), + ])); + column.push(""); + + let guidance = if self.is_git_repo { + "Since this folder is version controlled, you may wish to allow Codex to work in this folder without asking for approval." } else { - lines.push( - " Since this folder is not version controlled, we recommend requiring".into(), - ); - lines.push(" approval of all edits and commands.".into()); - } - lines.push("".into()); + "Since this folder is not version controlled, we recommend requiring approval of all edits and commands." + }; - let create_option = - |idx: usize, option: TrustDirectorySelection, text: &str| -> Line<'static> { - let is_selected = self.highlighted == option; - if is_selected { - Line::from(format!("> {}. {text}", idx + 1)).cyan() - } else { - Line::from(format!(" {}. {}", idx + 1, text)) - } - }; + column.push( + Paragraph::new(guidance.to_string()) + .wrap(Wrap { trim: true }) + .inset(Insets::tlbr(0, 2, 0, 0)), + ); + column.push(""); + let mut options: Vec<(&str, TrustDirectorySelection)> = Vec::new(); if self.is_git_repo { - lines.push(create_option( - 0, - TrustDirectorySelection::Trust, + options.push(( "Yes, allow Codex to work in this folder without asking for approval", + TrustDirectorySelection::Trust, )); - lines.push(create_option( - 1, - TrustDirectorySelection::DontTrust, + options.push(( "No, ask me to approve edits and commands", + TrustDirectorySelection::DontTrust, )); } else { - lines.push(create_option( - 0, - TrustDirectorySelection::Trust, + options.push(( "Allow Codex to work in this folder without asking for approval", + TrustDirectorySelection::Trust, )); - lines.push(create_option( - 1, - TrustDirectorySelection::DontTrust, + options.push(( "Require approval of edits and commands", + TrustDirectorySelection::DontTrust, )); } - lines.push("".into()); - if let Some(error) = &self.error { - lines.push(Line::from(format!(" {error}")).fg(Color::Red)); - lines.push("".into()); - } - // AE: Following styles.md, this should probably be Cyan because it's a user input tip. - // But leaving this for a future cleanup. - lines.push(Line::from(" Press Enter to continue").add_modifier(Modifier::DIM)); - Paragraph::new(lines) - .wrap(Wrap { trim: false }) - .render(area, buf); + for (idx, (text, selection)) in options.iter().enumerate() { + column.push(new_option_row( + idx, + text.to_string(), + self.highlighted == *selection, + )); + } + + column.push(""); + + if let Some(error) = &self.error { + column.push( + Paragraph::new(error.to_string()) + .red() + .wrap(Wrap { trim: true }) + .inset(Insets::tlbr(0, 2, 0, 0)), + ); + column.push(""); + } + + column.push( + Line::from(vec![ + "Press ".dim(), + key_hint::plain(KeyCode::Enter).into(), + " to continue".dim(), + ]) + .inset(Insets::tlbr(0, 2, 0, 0)), + ); + + column.render(area, buf); } } +fn new_option_row(index: usize, label: String, is_selected: bool) -> Box { + let prefix = if is_selected { + format!("› {}. ", index + 1) + } else { + format!(" {}. ", index + 1) + }; + + let mut style = Style::default(); + if is_selected { + style = style.cyan(); + } + + let mut row = RowRenderable::new(); + row.push(prefix.width() as u16, prefix.set_style(style)); + row.push( + u16::MAX, + Paragraph::new(label) + .style(style) + .wrap(Wrap { trim: false }), + ); + + row.into() +} + impl KeyboardHandler for TrustDirectoryWidget { fn handle_key_event(&mut self, key_event: KeyEvent) { if key_event.kind == KeyEventKind::Release { @@ -121,8 +157,8 @@ impl KeyboardHandler for TrustDirectoryWidget { KeyCode::Down | KeyCode::Char('j') => { self.highlighted = TrustDirectorySelection::DontTrust; } - KeyCode::Char('1') => self.handle_trust(), - KeyCode::Char('2') => self.handle_dont_trust(), + KeyCode::Char('1') | KeyCode::Char('y') => self.handle_trust(), + KeyCode::Char('2') | KeyCode::Char('n') => self.handle_dont_trust(), KeyCode::Enter => match self.highlighted { TrustDirectorySelection::Trust => self.handle_trust(), TrustDirectorySelection::DontTrust => self.handle_dont_trust(), @@ -161,12 +197,16 @@ impl TrustDirectoryWidget { #[cfg(test)] mod tests { + use crate::test_backend::VT100Backend; + use super::*; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; use crossterm::event::KeyModifiers; use pretty_assertions::assert_eq; + use ratatui::Terminal; + use std::path::PathBuf; #[test] @@ -191,4 +231,23 @@ mod tests { widget.handle_key_event(press); assert_eq!(widget.selection, Some(TrustDirectorySelection::DontTrust)); } + + #[test] + fn renders_snapshot_for_git_repo() { + let widget = TrustDirectoryWidget { + codex_home: PathBuf::from("."), + cwd: PathBuf::from("/workspace/project"), + is_git_repo: true, + selection: None, + highlighted: TrustDirectorySelection::Trust, + error: None, + }; + + let mut terminal = Terminal::new(VT100Backend::new(70, 14)).expect("terminal"); + terminal + .draw(|f| (&widget).render_ref(f.area(), f.buffer_mut())) + .expect("draw"); + + insta::assert_snapshot!(terminal.backend()); + } } diff --git a/codex-rs/tui/src/render/mod.rs b/codex-rs/tui/src/render/mod.rs index bf4fb553c5..a2e4920586 100644 --- a/codex-rs/tui/src/render/mod.rs +++ b/codex-rs/tui/src/render/mod.rs @@ -6,10 +6,10 @@ pub mod renderable; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct Insets { - pub left: u16, - pub top: u16, - pub right: u16, - pub bottom: u16, + left: u16, + top: u16, + right: u16, + bottom: u16, } impl Insets { diff --git a/codex-rs/tui/src/render/renderable.rs b/codex-rs/tui/src/render/renderable.rs index 62c2720685..df06449555 100644 --- a/codex-rs/tui/src/render/renderable.rs +++ b/codex-rs/tui/src/render/renderable.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::text::Line; +use ratatui::text::Span; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; @@ -45,6 +46,15 @@ impl Renderable for String { } } +impl<'a> Renderable for Span<'a> { + fn render(&self, area: Rect, buf: &mut Buffer) { + self.render_ref(area, buf); + } + fn desired_height(&self, _width: u16) -> u16 { + 1 + } +} + impl<'a> Renderable for Line<'a> { fn render(&self, area: Rect, buf: &mut Buffer) { WidgetRef::render_ref(self, area, buf); @@ -114,11 +124,64 @@ impl Renderable for ColumnRenderable { } impl ColumnRenderable { - pub fn new(children: impl IntoIterator>) -> Self { + pub fn new() -> Self { + Self::with(vec![]) + } + + pub fn with(children: impl IntoIterator>) -> Self { Self { children: children.into_iter().collect(), } } + + pub fn push(&mut self, child: impl Into>) { + self.children.push(child.into()); + } +} + +pub struct RowRenderable { + children: Vec<(u16, Box)>, +} + +impl Renderable for RowRenderable { + fn render(&self, area: Rect, buf: &mut Buffer) { + let mut x = area.x; + for (width, child) in &self.children { + let available_width = area.width.saturating_sub(x - area.x); + let child_area = Rect::new(x, area.y, (*width).min(available_width), area.height); + if child_area.is_empty() { + break; + } + child.render(child_area, buf); + x = x.saturating_add(*width); + } + } + fn desired_height(&self, width: u16) -> u16 { + let mut max_height = 0; + let mut width_remaining = width; + for (child_width, child) in &self.children { + let w = (*child_width).min(width_remaining); + if w == 0 { + break; + } + let height = child.desired_height(w); + if height > max_height { + max_height = height; + } + width_remaining = width_remaining.saturating_sub(w); + } + max_height + } +} + +impl RowRenderable { + pub fn new() -> Self { + Self { children: vec![] } + } + + pub fn push(&mut self, width: u16, child: impl Into>) { + self.children.push((width, child.into())); + } } pub struct InsetRenderable { @@ -146,3 +209,16 @@ impl InsetRenderable { } } } + +pub trait RenderableExt { + fn inset(self, insets: Insets) -> Box; +} + +impl>> RenderableExt for R { + fn inset(self, insets: Insets) -> Box { + Box::new(InsetRenderable { + child: self.into(), + insets, + }) + } +} diff --git a/codex-rs/tui/src/test_backend.rs b/codex-rs/tui/src/test_backend.rs index 52e7254711..a5460af2e0 100644 --- a/codex-rs/tui/src/test_backend.rs +++ b/codex-rs/tui/src/test_backend.rs @@ -91,7 +91,8 @@ impl Backend for VT100Backend { } fn size(&self) -> io::Result { - Ok(self.vt100().screen().size().into()) + let (rows, cols) = self.vt100().screen().size(); + Ok(Size::new(cols, rows)) } fn window_size(&mut self) -> io::Result { diff --git a/codex-rs/tui/src/wrapping.rs b/codex-rs/tui/src/wrapping.rs index 70ca2e46ca..3c3b775eb6 100644 --- a/codex-rs/tui/src/wrapping.rs +++ b/codex-rs/tui/src/wrapping.rs @@ -154,6 +154,7 @@ impl<'a> RtOptions<'a> { } } +#[must_use] pub(crate) fn word_wrap_line<'a, O>(line: &'a Line<'a>, width_or_options: O) -> Vec> where O: Into>, From 56296cad8275b96da0713ee63cda034763a9d42d Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Thu, 9 Oct 2025 14:01:45 -0700 Subject: [PATCH 08/17] tui: /diff mode wraps long lines (#4891) fixes a regression that stopped long lines from being wrapped when viewing `/diff`. --- codex-rs/tui/src/pager_overlay.rs | 33 ++++++++++++------- ...ests__static_overlay_wraps_long_lines.snap | 13 ++++++++ 2 files changed, 34 insertions(+), 12 deletions(-) create mode 100644 codex-rs/tui/src/snapshots/codex_tui__pager_overlay__tests__static_overlay_wraps_long_lines.snap diff --git a/codex-rs/tui/src/pager_overlay.rs b/codex-rs/tui/src/pager_overlay.rs index 9395fb71dd..84c325b6cb 100644 --- a/codex-rs/tui/src/pager_overlay.rs +++ b/codex-rs/tui/src/pager_overlay.rs @@ -26,6 +26,7 @@ use ratatui::widgets::Clear; use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; use ratatui::widgets::WidgetRef; +use ratatui::widgets::Wrap; pub(crate) enum Overlay { Transcript(TranscriptOverlay), @@ -329,9 +330,9 @@ struct CachedRenderable { } impl CachedRenderable { - fn new(renderable: Box) -> Self { + fn new(renderable: impl Into>) -> Self { Self { - renderable, + renderable: renderable.into(), height: std::cell::Cell::new(None), last_width: std::cell::Cell::new(None), } @@ -400,19 +401,19 @@ impl TranscriptOverlay { .flat_map(|(i, c)| { let mut v: Vec> = Vec::new(); let mut cell_renderable = if c.as_any().is::() { - Box::new(CachedRenderable::new(Box::new(CellRenderable { + Box::new(CachedRenderable::new(CellRenderable { cell: c.clone(), style: if highlight_cell == Some(i) { user_message_style().reversed() } else { user_message_style() }, - }))) as Box + })) as Box } else { - Box::new(CachedRenderable::new(Box::new(CellRenderable { + Box::new(CachedRenderable::new(CellRenderable { cell: c.clone(), style: Style::default(), - }))) as Box + })) as Box }; if !c.is_stream_continuation() && i > 0 { cell_renderable = Box::new(InsetRenderable::new( @@ -496,12 +497,8 @@ pub(crate) struct StaticOverlay { impl StaticOverlay { pub(crate) fn with_title(lines: Vec>, title: String) -> Self { - Self::with_renderables( - vec![Box::new(CachedRenderable::new(Box::new(Paragraph::new( - Text::from(lines), - ))))], - title, - ) + let paragraph = Paragraph::new(Text::from(lines)).wrap(Wrap { trim: false }); + Self::with_renderables(vec![Box::new(CachedRenderable::new(paragraph))], title) } pub(crate) fn with_renderables(renderables: Vec>, title: String) -> Self { @@ -812,6 +809,18 @@ mod tests { assert_snapshot!(term.backend()); } + #[test] + fn static_overlay_wraps_long_lines() { + let mut overlay = StaticOverlay::with_title( + vec!["a very long line that should wrap when rendered within a narrow pager overlay width".into()], + "S T A T I C".to_string(), + ); + let mut term = Terminal::new(TestBackend::new(24, 8)).expect("term"); + term.draw(|f| overlay.render(f.area(), f.buffer_mut())) + .expect("draw"); + assert_snapshot!(term.backend()); + } + #[test] fn pager_view_content_height_counts_renderables() { let pv = PagerView::new( diff --git a/codex-rs/tui/src/snapshots/codex_tui__pager_overlay__tests__static_overlay_wraps_long_lines.snap b/codex-rs/tui/src/snapshots/codex_tui__pager_overlay__tests__static_overlay_wraps_long_lines.snap new file mode 100644 index 0000000000..2dfab2aa7d --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__pager_overlay__tests__static_overlay_wraps_long_lines.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/pager_overlay.rs +assertion_line: 798 +expression: term.backend() +--- +"/ S T A T I C / / / / / " +"a very long line that " +"should wrap when " +"rendered within a narrow" +"─────────────────── 0% ─" +" ↑/↓ to scroll pgup/pg" +" q to quit " +" " From ddcaf3dccd7a817b6307bff61a4604c4dff27621 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Thu, 9 Oct 2025 15:07:04 -0700 Subject: [PATCH 09/17] tui: fix crash when alt+bksp past unicode nbsp (#5016) notably, screenshot filenames on macOS by default contain U+202F right before the "AM/PM" part of the filename. --- codex-rs/tui/src/bottom_pane/textarea.rs | 35 +++++++++++++++++------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 269fa345e0..6687793979 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -799,16 +799,22 @@ impl TextArea { } pub(crate) fn beginning_of_previous_word(&self) -> usize { - if let Some(first_non_ws) = self.text[..self.cursor_pos].rfind(|c: char| !c.is_whitespace()) - { - let candidate = self.text[..first_non_ws] - .rfind(|c: char| c.is_whitespace()) - .map(|i| i + 1) - .unwrap_or(0); - self.adjust_pos_out_of_elements(candidate, true) - } else { - 0 - } + let prefix = &self.text[..self.cursor_pos]; + let Some((first_non_ws_idx, _)) = prefix + .char_indices() + .rev() + .find(|&(_, ch)| !ch.is_whitespace()) + else { + return 0; + }; + let before = &prefix[..first_non_ws_idx]; + let candidate = before + .char_indices() + .rev() + .find(|&(_, ch)| ch.is_whitespace()) + .map(|(idx, ch)| idx + ch.len_utf8()) + .unwrap_or(0); + self.adjust_pos_out_of_elements(candidate, true) } pub(crate) fn end_of_next_word(&self) -> usize { @@ -1262,6 +1268,15 @@ mod tests { assert_eq!(t.cursor(), 6); } + #[test] + fn delete_backward_word_handles_narrow_no_break_space() { + let mut t = ta_with("32\u{202F}AM"); + t.set_cursor(t.text().len()); + t.input(KeyEvent::new(KeyCode::Backspace, KeyModifiers::ALT)); + pretty_assertions::assert_eq!(t.text(), "32\u{202F}"); + pretty_assertions::assert_eq!(t.cursor(), t.text().len()); + } + #[test] fn delete_forward_word_with_without_alt_modifier() { let mut t = ta_with("hello world"); From f98fa85b448d4cb0d6e0b5cb04e7fec2876ff2bf Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 10 Oct 2025 10:07:14 +0100 Subject: [PATCH 10/17] feat: message when stream get correctly resumed (#4988) Screenshot 2025-10-09 at 17 44 16 --- codex-rs/core/src/codex.rs | 4 +-- codex-rs/core/src/codex/compact.rs | 4 +-- codex-rs/protocol/src/protocol.rs | 5 ++++ codex-rs/tui/src/chatwidget.rs | 27 ++++++++++++++++++--- codex-rs/tui/src/chatwidget/tests.rs | 21 ++++++++++------ codex-rs/tui/src/history_cell.rs | 5 ---- codex-rs/tui/src/status_indicator_widget.rs | 5 ++++ 7 files changed, 49 insertions(+), 22 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index cae47cb322..fe352f0103 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2008,9 +2008,7 @@ async fn run_turn( // at a seemingly frozen screen. sess.notify_stream_error( &sub_id, - format!( - "stream error: {e}; retrying {retries}/{max_retries} in {delay:?}…" - ), + format!("Re-connecting... {retries}/{max_retries}"), ) .await; diff --git a/codex-rs/core/src/codex/compact.rs b/codex-rs/core/src/codex/compact.rs index d35df99c8d..d43e3abcbb 100644 --- a/codex-rs/core/src/codex/compact.rs +++ b/codex-rs/core/src/codex/compact.rs @@ -136,9 +136,7 @@ async fn run_compact_task_inner( let delay = backoff(retries); sess.notify_stream_error( &sub_id, - format!( - "stream error: {e}; retrying {retries}/{max_retries} in {delay:?}…" - ), + format!("Re-connecting... {retries}/{max_retries}"), ) .await; tokio::time::sleep(delay).await; diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 1ae32e5122..b73909473d 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1203,6 +1203,11 @@ pub struct StreamErrorEvent { pub message: String, } +#[derive(Debug, Clone, Deserialize, Serialize, TS)] +pub struct StreamInfoEvent { + pub message: String, +} + #[derive(Debug, Clone, Deserialize, Serialize, TS)] pub struct PatchApplyBeginEvent { /// Identifier so this can be paired with the PatchApplyEnd event. diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 973b0e3ba2..aeaf239f32 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -240,6 +240,10 @@ pub(crate) struct ChatWidget { reasoning_buffer: String, // Accumulates full reasoning content for transcript-only recording full_reasoning_buffer: String, + // Current status header shown in the status indicator. + current_status_header: String, + // Previous status header to restore after a transient stream retry. + retry_status_header: Option, conversation_id: Option, frame_requester: FrameRequester, // Whether to include the initial welcome banner on session configured @@ -303,6 +307,14 @@ impl ChatWidget { } } + fn set_status_header(&mut self, header: String) { + if self.current_status_header == header { + return; + } + self.current_status_header = header.clone(); + self.bottom_pane.update_status_header(header); + } + // --- Small event handlers --- fn on_session_configured(&mut self, event: codex_core::protocol::SessionConfiguredEvent) { self.bottom_pane @@ -352,7 +364,7 @@ impl ChatWidget { if let Some(header) = extract_first_bold(&self.reasoning_buffer) { // Update the shimmer header to the extracted reasoning chunk header. - self.bottom_pane.update_status_header(header); + self.set_status_header(header); } else { // Fallback while we don't yet have a bold header: leave existing header as-is. } @@ -386,6 +398,8 @@ impl ChatWidget { fn on_task_started(&mut self) { self.bottom_pane.clear_ctrl_c_quit_hint(); self.bottom_pane.set_task_running(true); + self.retry_status_header = None; + self.set_status_header(String::from("Working")); self.full_reasoning_buffer.clear(); self.reasoning_buffer.clear(); self.request_redraw(); @@ -621,9 +635,10 @@ impl ChatWidget { } fn on_stream_error(&mut self, message: String) { - // Show stream errors in the transcript so users see retry/backoff info. - self.add_to_history(history_cell::new_stream_error_event(message)); - self.request_redraw(); + if self.retry_status_header.is_none() { + self.retry_status_header = Some(self.current_status_header.clone()); + } + self.set_status_header(message); } /// Periodic tick to commit at most one queued line to history with a small delay, @@ -928,6 +943,8 @@ impl ChatWidget { interrupts: InterruptManager::new(), reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), + current_status_header: String::from("Working"), + retry_status_header: None, conversation_id: None, queued_user_messages: VecDeque::new(), show_welcome_banner: true, @@ -991,6 +1008,8 @@ impl ChatWidget { interrupts: InterruptManager::new(), reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), + current_status_header: String::from("Working"), + retry_status_header: None, conversation_id: None, queued_user_messages: VecDeque::new(), show_welcome_banner: true, diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 2055445059..d4cf4c7b20 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -276,6 +276,8 @@ fn make_chatwidget_manual() -> ( interrupts: InterruptManager::new(), reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), + current_status_header: String::from("Working"), + retry_status_header: None, conversation_id: None, frame_requester: FrameRequester::test_dummy(), show_welcome_banner: true, @@ -2044,9 +2046,10 @@ fn plan_update_renders_history_cell() { } #[test] -fn stream_error_is_rendered_to_history() { +fn stream_error_updates_status_indicator() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); - let msg = "stream error: stream disconnected before completion: idle timeout waiting for SSE; retrying 1/5 in 211ms…"; + chat.bottom_pane.set_task_running(true); + let msg = "Re-connecting... 2/5"; chat.handle_codex_event(Event { id: "sub-1".into(), msg: EventMsg::StreamError(StreamErrorEvent { @@ -2055,11 +2058,15 @@ fn stream_error_is_rendered_to_history() { }); let cells = drain_insert_history(&mut rx); - assert!(!cells.is_empty(), "expected a history cell for StreamError"); - let blob = lines_to_single_string(cells.last().unwrap()); - assert!(blob.contains('⚠')); - assert!(blob.contains("stream error:")); - assert!(blob.contains("idle timeout waiting for SSE")); + assert!( + cells.is_empty(), + "expected no history cell for StreamError event" + ); + let status = chat + .bottom_pane + .status_widget() + .expect("status indicator should be visible"); + assert_eq!(status.header(), msg); } #[test] diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 18a6c72d24..aa0ae35d92 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -1062,11 +1062,6 @@ pub(crate) fn new_error_event(message: String) -> PlainHistoryCell { PlainHistoryCell { lines } } -pub(crate) fn new_stream_error_event(message: String) -> PlainHistoryCell { - let lines: Vec> = vec![vec![padded_emoji("⚠️").into(), message.dim()].into()]; - PlainHistoryCell { lines } -} - /// Render a user‑friendly plan update styled like a checkbox todo list. pub(crate) fn new_plan_update(update: UpdatePlanArgs) -> PlanUpdateCell { let UpdatePlanArgs { explanation, plan } = update; diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index ce4f6eabd8..4bcc5cf267 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -103,6 +103,11 @@ impl StatusIndicatorWidget { } } + #[cfg(test)] + pub(crate) fn header(&self) -> &str { + &self.header + } + /// Replace the queued messages displayed beneath the header. pub(crate) fn set_queued_messages(&mut self, queued: Vec) { self.queued_messages = queued; From 85e735797366b7272cd36cd81b7fd5aff8418d86 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 10 Oct 2025 14:57:47 +0100 Subject: [PATCH 11/17] fix: workflow cache (#5050) Decouple cache saving to fix the `verify` steps never being run due to a cache saving issue --- .github/workflows/rust-ci.yml | 42 +++++++++++++++++++++-- codex-rs/core/tests/suite/unified_exec.rs | 2 +- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index 846408f3a8..cbf62f72f0 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -148,15 +148,26 @@ jobs: targets: ${{ matrix.target }} components: clippy - - uses: actions/cache@v4 + # Explicit cache restore: split cargo home vs target, so we can + # avoid caching the large target dir on the gnu-dev job. + - name: Restore cargo home cache + id: cache_cargo_home_restore + uses: actions/cache/restore@v4 with: path: | ~/.cargo/bin/ ~/.cargo/registry/index/ ~/.cargo/registry/cache/ ~/.cargo/git/db/ - ${{ github.workspace }}/codex-rs/target/ - key: cargo-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }} + key: cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }} + + - name: Restore target cache (except gnu-dev) + id: cache_target_restore + if: ${{ !(matrix.target == 'x86_64-unknown-linux-gnu' && matrix.profile != 'release') }} + uses: actions/cache/restore@v4 + with: + path: ${{ github.workspace }}/codex-rs/target/ + key: cargo-target-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }} - if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl'}} name: Install musl build tools @@ -194,6 +205,31 @@ jobs: env: RUST_BACKTRACE: 1 + # Save caches explicitly; make non-fatal so cache packaging + # never fails the overall job. Only save when key wasn't hit. + - name: Save cargo home cache + if: always() && !cancelled() && steps.cache_cargo_home_restore.outputs.cache-hit != 'true' + continue-on-error: true + uses: actions/cache/save@v4 + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + key: cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }} + + - name: Save target cache (except gnu-dev) + if: >- + always() && !cancelled() && + (steps.cache_target_restore.outputs.cache-hit != 'true') && + !(matrix.target == 'x86_64-unknown-linux-gnu' && matrix.profile != 'release') + continue-on-error: true + uses: actions/cache/save@v4 + with: + path: ${{ github.workspace }}/codex-rs/target/ + key: cargo-target-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }} + # Fail the job if any of the previous steps failed. - name: verify all steps passed if: | diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index b8c0fadbbe..cfa96dd75c 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -213,7 +213,7 @@ PY let second_args = serde_json::json!({ "input": Vec::::new(), "session_id": "0", - "timeout_ms": 800, + "timeout_ms": 2_000, }); let responses = vec![ From 961ed31901b00e60241e54c0b2f1646048473043 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 10 Oct 2025 15:35:28 +0100 Subject: [PATCH 12/17] feat: make shortcut works even with capslock (#5049) Shortcut where not working in caps-lock. Fixing this --- codex-rs/core/tests/suite/shell_serialization.rs | 2 +- codex-rs/tui/src/chatwidget.rs | 12 ++++++------ codex-rs/tui/src/chatwidget/tests.rs | 12 ++++++++++++ 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/codex-rs/core/tests/suite/shell_serialization.rs b/codex-rs/core/tests/suite/shell_serialization.rs index 67fc2786ca..fd9c26d882 100644 --- a/codex-rs/core/tests/suite/shell_serialization.rs +++ b/codex-rs/core/tests/suite/shell_serialization.rs @@ -213,7 +213,7 @@ async fn shell_output_reserializes_truncated_content() -> Result<()> { let call_id = "shell-truncated"; let args = json!({ "command": ["/bin/sh", "-c", "seq 1 400"], - "timeout_ms": 1_000, + "timeout_ms": 5_000, }); let responses = vec![ sse(vec![ diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index aeaf239f32..787e098093 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1034,20 +1034,20 @@ impl ChatWidget { pub(crate) fn handle_key_event(&mut self, key_event: KeyEvent) { match key_event { KeyEvent { - code: KeyCode::Char('c'), - modifiers: crossterm::event::KeyModifiers::CONTROL, + code: KeyCode::Char(c), + modifiers, kind: KeyEventKind::Press, .. - } => { + } if modifiers.contains(KeyModifiers::CONTROL) && c.eq_ignore_ascii_case(&'c') => { self.on_ctrl_c(); return; } KeyEvent { - code: KeyCode::Char('v'), - modifiers: KeyModifiers::CONTROL, + code: KeyCode::Char(c), + modifiers, kind: KeyEventKind::Press, .. - } => { + } if modifiers.contains(KeyModifiers::CONTROL) && c.eq_ignore_ascii_case(&'v') => { if let Ok((path, info)) = paste_image_to_temp_png() { self.attach_image(path, info.width, info.height, info.encoded_format.label()); } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index d4cf4c7b20..3bfdf243bc 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -676,6 +676,18 @@ fn streaming_final_answer_keeps_task_running_state() { assert!(chat.bottom_pane.ctrl_c_quit_hint_visible()); } +#[test] +fn ctrl_c_shutdown_ignores_caps_lock() { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(); + + chat.handle_key_event(KeyEvent::new(KeyCode::Char('C'), KeyModifiers::CONTROL)); + + match op_rx.try_recv() { + Ok(Op::Shutdown) => {} + other => panic!("expected Op::Shutdown, got {other:?}"), + } +} + #[test] fn exec_history_cell_shows_working_then_completed() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); From 90af046c5c08da705cf7a99adc7b7b4ffd7e8daf Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Fri, 10 Oct 2025 09:56:18 -0700 Subject: [PATCH 13/17] tui: include the image name in the textarea placeholder (#5056) Fixes #5013 --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 46 ++++++++++++++----- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 7aa6e66958..3cd38e8de9 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -319,8 +319,12 @@ impl ChatComposer { } /// Attempt to start a burst by retro-capturing recent chars before the cursor. - pub fn attach_image(&mut self, path: PathBuf, width: u32, height: u32, format_label: &str) { - let placeholder = format!("[image {width}x{height} {format_label}]"); + pub fn attach_image(&mut self, path: PathBuf, width: u32, height: u32, _format_label: &str) { + let file_label = path + .file_name() + .map(|name| name.to_string_lossy().into_owned()) + .unwrap_or_else(|| "image".to_string()); + let placeholder = format!("[{file_label} {width}x{height}]"); // Insert as an element to match large paste placeholder behavior: // styled distinctly and treated atomically for cursor/mutations. self.textarea.insert_element(&placeholder); @@ -2581,7 +2585,7 @@ mod tests { let (result, _) = composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); match result { - InputResult::Submitted(text) => assert_eq!(text, "[image 32x16 PNG] hi"), + InputResult::Submitted(text) => assert_eq!(text, "[image1.png 32x16] hi"), _ => panic!("expected Submitted"), } let imgs = composer.take_recent_submission_images(); @@ -2604,7 +2608,7 @@ mod tests { let (result, _) = composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); match result { - InputResult::Submitted(text) => assert_eq!(text, "[image 10x5 PNG]"), + InputResult::Submitted(text) => assert_eq!(text, "[image2.png 10x5]"), _ => panic!("expected Submitted"), } let imgs = composer.take_recent_submission_images(); @@ -2677,7 +2681,12 @@ mod tests { composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE)); assert_eq!(composer.attached_images.len(), 1); - assert!(composer.textarea.text().starts_with("[image 10x5 PNG]")); + assert!( + composer + .textarea + .text() + .starts_with("[image_multibyte.png 10x5]") + ); } #[test] @@ -2700,21 +2709,31 @@ mod tests { composer.handle_paste(" ".into()); composer.attach_image(path2.clone(), 10, 5, "PNG"); - let ph = composer.attached_images[0].placeholder.clone(); + let placeholder1 = composer.attached_images[0].placeholder.clone(); + let placeholder2 = composer.attached_images[1].placeholder.clone(); let text = composer.textarea.text().to_string(); - let start1 = text.find(&ph).expect("first placeholder present"); - let end1 = start1 + ph.len(); + let start1 = text.find(&placeholder1).expect("first placeholder present"); + let end1 = start1 + placeholder1.len(); composer.textarea.set_cursor(end1); // Backspace should delete the first placeholder and its mapping. composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE)); let new_text = composer.textarea.text().to_string(); - assert_eq!(1, new_text.matches(&ph).count(), "one placeholder remains"); + assert_eq!( + 0, + new_text.matches(&placeholder1).count(), + "first placeholder removed" + ); + assert_eq!( + 1, + new_text.matches(&placeholder2).count(), + "second placeholder remains" + ); assert_eq!( vec![AttachedImage { path: path2, - placeholder: "[image 10x5 PNG]".to_string() + placeholder: "[image_dup2.png 10x5]".to_string() }], composer.attached_images, "one image mapping remains" @@ -2741,7 +2760,12 @@ mod tests { let needs_redraw = composer.handle_paste(tmp_path.to_string_lossy().to_string()); assert!(needs_redraw); - assert!(composer.textarea.text().starts_with("[image 3x2 PNG] ")); + assert!( + composer + .textarea + .text() + .starts_with("[codex_tui_test_paste_image.png 3x2] ") + ); let imgs = composer.take_recent_submission_images(); assert_eq!(imgs, vec![tmp_path]); From 26f7c46856060f440615645bdf2faf9e62c74e72 Mon Sep 17 00:00:00 2001 From: Shijie Rao Date: Fri, 10 Oct 2025 17:23:41 -0700 Subject: [PATCH 14/17] fixes #5011: update mcp server doc (#5014) --- docs/config.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/config.md b/docs/config.md index fba8a3a21d..2ac7aba6b7 100644 --- a/docs/config.md +++ b/docs/config.md @@ -370,9 +370,8 @@ API_KEY = "value" experimental_use_rmcp_client = true [mcp_servers.figma] url = "https://mcp.linear.app/mcp" -# Optional bearer token to be passed into an `Authorization: Bearer ` header -# Use this with caution because the token is in plaintext and can be read by Codex itself. -bearer_token = "" +# Optional environment variable containing a bearer token to use for auth +bearer_token_env_var = "" ``` For oauth login, you must enable `experimental_use_rmcp_client = true` and then run `codex mcp login server_name` From 5346cc422dd053717c6f22ab20ffbd0aa5c6594b Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 14 Oct 2025 09:47:20 +0100 Subject: [PATCH 15/17] feat: discard prompt starting with a slash (#5048) This is does not consider lines starting with a space or containing multiple `/` as commands Screenshot 2025-10-13 at 10 00 08 --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 3cd38e8de9..5b63a410bb 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -962,6 +962,7 @@ impl ChatComposer { } let mut text = self.textarea.text().to_string(); let original_input = text.clone(); + let input_starts_with_space = original_input.starts_with(' '); self.textarea.set_text(""); // Replace all pending pastes in the text @@ -975,6 +976,35 @@ impl ChatComposer { // If there is neither text nor attachments, suppress submission entirely. let has_attachments = !self.attached_images.is_empty(); text = text.trim().to_string(); + if let Some((name, _rest)) = parse_slash_name(&text) { + let treat_as_plain_text = input_starts_with_space || name.contains('/'); + if !treat_as_plain_text { + let is_builtin = built_in_slash_commands() + .into_iter() + .any(|(command_name, _)| command_name == name); + let prompt_prefix = format!("{PROMPTS_CMD_PREFIX}:"); + let is_known_prompt = name + .strip_prefix(&prompt_prefix) + .map(|prompt_name| { + self.custom_prompts + .iter() + .any(|prompt| prompt.name == prompt_name) + }) + .unwrap_or(false); + if !is_builtin && !is_known_prompt { + let message = format!( + r#"Unrecognized command '/{name}'. Type "/" for a list of supported commands."# + ); + self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( + history_cell::new_info_event(message, None), + ))); + self.textarea.set_text(&original_input); + self.textarea.set_cursor(original_input.len()); + return (InputResult::None, true); + } + } + } + let expanded_prompt = match expand_custom_prompt(&text, &self.custom_prompts) { Ok(expanded) => expanded, Err(err) => { @@ -2877,6 +2907,76 @@ mod tests { assert!(composer.textarea.is_empty()); } + #[test] + fn slash_path_input_submits_without_command_error() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, mut rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + composer + .textarea + .set_text("/Users/example/project/src/main.rs"); + + let (result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + if let InputResult::Submitted(text) = result { + assert_eq!(text, "/Users/example/project/src/main.rs"); + } else { + panic!("expected Submitted"); + } + assert!(composer.textarea.is_empty()); + match rx.try_recv() { + Ok(event) => panic!("unexpected event: {event:?}"), + Err(tokio::sync::mpsc::error::TryRecvError::Empty) => {} + Err(err) => panic!("unexpected channel state: {err:?}"), + } + } + + #[test] + fn slash_with_leading_space_submits_as_text() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, mut rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + composer.textarea.set_text(" /this-looks-like-a-command"); + + let (result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + if let InputResult::Submitted(text) = result { + assert_eq!(text, "/this-looks-like-a-command"); + } else { + panic!("expected Submitted"); + } + assert!(composer.textarea.is_empty()); + match rx.try_recv() { + Ok(event) => panic!("unexpected event: {event:?}"), + Err(tokio::sync::mpsc::error::TryRecvError::Empty) => {} + Err(err) => panic!("unexpected channel state: {err:?}"), + } + } + #[test] fn custom_prompt_invalid_args_reports_error() { let (tx, mut rx) = unbounded_channel::(); From 268a10f917ac43b3e66c03ee0aa1f179f754ec9a Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 14 Oct 2025 16:17:00 +0100 Subject: [PATCH 16/17] feat: add header for task kind (#5142) Add a header in the responses API request for the task kind (compact, review, ...) for observability purpose The header name is `codex-task-type` --- codex-rs/core/src/client.rs | 21 ++++- codex-rs/core/src/codex.rs | 12 ++- codex-rs/core/src/codex/compact.rs | 7 +- codex-rs/core/src/state/turn.rs | 22 +++++ codex-rs/core/src/tasks/regular.rs | 2 +- codex-rs/core/src/tasks/review.rs | 2 +- codex-rs/core/tests/responses_headers.rs | 102 +++++++++++++++++++++++ 7 files changed, 161 insertions(+), 7 deletions(-) create mode 100644 codex-rs/core/tests/responses_headers.rs diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 3ea2ca79b5..a8dcb7259a 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -47,6 +47,7 @@ use crate::openai_tools::create_tools_json_for_responses_api; use crate::protocol::RateLimitSnapshot; use crate::protocol::RateLimitWindow; use crate::protocol::TokenUsage; +use crate::state::TaskKind; use crate::token_data::PlanType; use crate::util::backoff; use codex_otel::otel_event_manager::OtelEventManager; @@ -123,8 +124,16 @@ impl ModelClient { /// the provider config. Public callers always invoke `stream()` – the /// specialised helpers are private to avoid accidental misuse. pub async fn stream(&self, prompt: &Prompt) -> Result { + self.stream_with_task_kind(prompt, TaskKind::Regular).await + } + + pub(crate) async fn stream_with_task_kind( + &self, + prompt: &Prompt, + task_kind: TaskKind, + ) -> Result { match self.provider.wire_api { - WireApi::Responses => self.stream_responses(prompt).await, + WireApi::Responses => self.stream_responses(prompt, task_kind).await, WireApi::Chat => { // Create the raw streaming connection first. let response_stream = stream_chat_completions( @@ -165,7 +174,11 @@ impl ModelClient { } /// Implementation for the OpenAI *Responses* experimental API. - async fn stream_responses(&self, prompt: &Prompt) -> Result { + async fn stream_responses( + &self, + prompt: &Prompt, + task_kind: TaskKind, + ) -> Result { if let Some(path) = &*CODEX_RS_SSE_FIXTURE { // short circuit for tests warn!(path, "Streaming from fixture"); @@ -244,7 +257,7 @@ impl ModelClient { let max_attempts = self.provider.request_max_retries(); for attempt in 0..=max_attempts { match self - .attempt_stream_responses(attempt, &payload_json, &auth_manager) + .attempt_stream_responses(attempt, &payload_json, &auth_manager, task_kind) .await { Ok(stream) => { @@ -272,6 +285,7 @@ impl ModelClient { attempt: u64, payload_json: &Value, auth_manager: &Option>, + task_kind: TaskKind, ) -> std::result::Result { // Always fetch the latest auth in case a prior attempt refreshed the token. let auth = auth_manager.as_ref().and_then(|m| m.auth()); @@ -294,6 +308,7 @@ impl ModelClient { .header("conversation_id", self.conversation_id.to_string()) .header("session_id", self.conversation_id.to_string()) .header(reqwest::header::ACCEPT, "text/event-stream") + .header("Codex-Task-Type", task_kind.header_value()) .json(payload_json); if let Some(auth) = auth.as_ref() diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index fe352f0103..22b34b1cd0 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -99,6 +99,7 @@ use crate::rollout::RolloutRecorderParams; use crate::shell; use crate::state::ActiveTurn; use crate::state::SessionServices; +use crate::state::TaskKind; use crate::tasks::CompactTask; use crate::tasks::RegularTask; use crate::tasks::ReviewTask; @@ -1634,6 +1635,7 @@ pub(crate) async fn run_task( turn_context: Arc, sub_id: String, input: Vec, + task_kind: TaskKind, ) -> Option { if input.is_empty() { return None; @@ -1717,6 +1719,7 @@ pub(crate) async fn run_task( Arc::clone(&turn_diff_tracker), sub_id.clone(), turn_input, + task_kind, ) .await { @@ -1942,6 +1945,7 @@ async fn run_turn( turn_diff_tracker: SharedTurnDiffTracker, sub_id: String, input: Vec, + task_kind: TaskKind, ) -> CodexResult { let mcp_tools = sess.services.mcp_connection_manager.list_all_tools(); let router = Arc::new(ToolRouter::from_config( @@ -1971,6 +1975,7 @@ async fn run_turn( Arc::clone(&turn_diff_tracker), &sub_id, &prompt, + task_kind, ) .await { @@ -2044,6 +2049,7 @@ async fn try_run_turn( turn_diff_tracker: SharedTurnDiffTracker, sub_id: &str, prompt: &Prompt, + task_kind: TaskKind, ) -> CodexResult { // call_ids that are part of this response. let completed_call_ids = prompt @@ -2109,7 +2115,11 @@ async fn try_run_turn( summary: turn_context.client.get_reasoning_summary(), }); sess.persist_rollout_items(&[rollout_item]).await; - let mut stream = turn_context.client.clone().stream(&prompt).await?; + let mut stream = turn_context + .client + .clone() + .stream_with_task_kind(prompt.as_ref(), task_kind) + .await?; let tool_runtime = ToolCallRuntime::new( Arc::clone(&router), diff --git a/codex-rs/core/src/codex/compact.rs b/codex-rs/core/src/codex/compact.rs index d43e3abcbb..93bbfa79c6 100644 --- a/codex-rs/core/src/codex/compact.rs +++ b/codex-rs/core/src/codex/compact.rs @@ -16,6 +16,7 @@ use crate::protocol::InputItem; use crate::protocol::InputMessageKind; use crate::protocol::TaskStartedEvent; use crate::protocol::TurnContextItem; +use crate::state::TaskKind; use crate::truncate::truncate_middle; use crate::util::backoff; use askama::Template; @@ -258,7 +259,11 @@ async fn drain_to_completed( sub_id: &str, prompt: &Prompt, ) -> CodexResult<()> { - let mut stream = turn_context.client.clone().stream(prompt).await?; + let mut stream = turn_context + .client + .clone() + .stream_with_task_kind(prompt, TaskKind::Compact) + .await?; loop { let maybe_event = stream.next().await; let Some(event) = maybe_event else { diff --git a/codex-rs/core/src/state/turn.rs b/codex-rs/core/src/state/turn.rs index f715d5481e..89af13a1a5 100644 --- a/codex-rs/core/src/state/turn.rs +++ b/codex-rs/core/src/state/turn.rs @@ -34,6 +34,16 @@ pub(crate) enum TaskKind { Compact, } +impl TaskKind { + pub(crate) fn header_value(self) -> &'static str { + match self { + TaskKind::Regular => "standard", + TaskKind::Review => "review", + TaskKind::Compact => "compact", + } + } +} + #[derive(Clone)] pub(crate) struct RunningTask { pub(crate) handle: AbortHandle, @@ -113,3 +123,15 @@ impl ActiveTurn { } } } + +#[cfg(test)] +mod tests { + use super::TaskKind; + + #[test] + fn header_value_matches_expected_labels() { + assert_eq!(TaskKind::Regular.header_value(), "standard"); + assert_eq!(TaskKind::Review.header_value(), "review"); + assert_eq!(TaskKind::Compact.header_value(), "compact"); + } +} diff --git a/codex-rs/core/src/tasks/regular.rs b/codex-rs/core/src/tasks/regular.rs index 9d24099746..b3758d5fc6 100644 --- a/codex-rs/core/src/tasks/regular.rs +++ b/codex-rs/core/src/tasks/regular.rs @@ -27,6 +27,6 @@ impl SessionTask for RegularTask { input: Vec, ) -> Option { let sess = session.clone_session(); - run_task(sess, ctx, sub_id, input).await + run_task(sess, ctx, sub_id, input, TaskKind::Regular).await } } diff --git a/codex-rs/core/src/tasks/review.rs b/codex-rs/core/src/tasks/review.rs index 047a2f40e2..cec9243234 100644 --- a/codex-rs/core/src/tasks/review.rs +++ b/codex-rs/core/src/tasks/review.rs @@ -28,7 +28,7 @@ impl SessionTask for ReviewTask { input: Vec, ) -> Option { let sess = session.clone_session(); - run_task(sess, ctx, sub_id, input).await + run_task(sess, ctx, sub_id, input, TaskKind::Review).await } async fn abort(&self, session: Arc, sub_id: &str) { diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs new file mode 100644 index 0000000000..19967b06ea --- /dev/null +++ b/codex-rs/core/tests/responses_headers.rs @@ -0,0 +1,102 @@ +use std::sync::Arc; + +use codex_app_server_protocol::AuthMode; +use codex_core::ContentItem; +use codex_core::ModelClient; +use codex_core::ModelProviderInfo; +use codex_core::Prompt; +use codex_core::ResponseEvent; +use codex_core::ResponseItem; +use codex_core::WireApi; +use codex_otel::otel_event_manager::OtelEventManager; +use codex_protocol::ConversationId; +use core_test_support::load_default_config_for_test; +use core_test_support::responses; +use futures::StreamExt; +use tempfile::TempDir; +use wiremock::matchers::header; + +#[tokio::test] +async fn responses_stream_includes_task_type_header() { + core_test_support::skip_if_no_network!(); + + let server = responses::start_mock_server().await; + let response_body = responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_completed("resp-1"), + ]); + + let request_recorder = responses::mount_sse_once_match( + &server, + header("Codex-Task-Type", "standard"), + response_body, + ) + .await; + + let provider = ModelProviderInfo { + name: "mock".into(), + base_url: Some(format!("{}/v1", server.uri())), + env_key: None, + env_key_instructions: None, + wire_api: WireApi::Responses, + query_params: None, + http_headers: None, + env_http_headers: None, + request_max_retries: Some(0), + stream_max_retries: Some(0), + stream_idle_timeout_ms: Some(5_000), + requires_openai_auth: false, + }; + + let codex_home = TempDir::new().expect("failed to create TempDir"); + let mut config = load_default_config_for_test(&codex_home); + config.model_provider_id = provider.name.clone(); + config.model_provider = provider.clone(); + let effort = config.model_reasoning_effort; + let summary = config.model_reasoning_summary; + let config = Arc::new(config); + + let conversation_id = ConversationId::new(); + + let otel_event_manager = OtelEventManager::new( + conversation_id, + config.model.as_str(), + config.model_family.slug.as_str(), + None, + Some(AuthMode::ChatGPT), + false, + "test".to_string(), + ); + + let client = ModelClient::new( + Arc::clone(&config), + None, + otel_event_manager, + provider, + effort, + summary, + conversation_id, + ); + + let mut prompt = Prompt::default(); + prompt.input = vec![ResponseItem::Message { + id: None, + role: "user".into(), + content: vec![ContentItem::InputText { + text: "hello".into(), + }], + }]; + + let mut stream = client.stream(&prompt).await.expect("stream failed"); + while let Some(event) = stream.next().await { + if matches!(event, Ok(ResponseEvent::Completed { .. })) { + break; + } + } + + let request = request_recorder.single_request(); + assert_eq!( + request.header("Codex-Task-Type").as_deref(), + Some("standard") + ); +} From 57584d6f341ead766242a59186e168c7ab299587 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 14 Oct 2025 18:15:47 +0100 Subject: [PATCH 17/17] fix: the 7 omitted lines issue (#5141) Before, the CLI was always showing `... +7 lines` (with the 7 constant) due to a double truncation Screenshot 2025-10-13 at 10 28 11 --- ...et__tests__binary_size_ideal_response.snap | 3 +- codex-rs/tui/src/exec_cell/render.rs | 71 +++++++++++++++---- codex-rs/tui/src/history_cell.rs | 5 +- 3 files changed, 62 insertions(+), 17 deletions(-) diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap index e3121774f4..77738439a1 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap @@ -1,6 +1,5 @@ --- source: tui/src/chatwidget/tests.rs -assertion_line: 1152 expression: "lines[start_idx..].join(\"\\n\")" --- • I need to check the codex-rs repository to explain why the project's binaries @@ -33,7 +32,7 @@ expression: "lines[start_idx..].join(\"\\n\")" │ … +1 lines └ --- ansi-escape/Cargo.toml [package] - … +7 lines + … +243 lines ] } tracing = { version diff --git a/codex-rs/tui/src/exec_cell/render.rs b/codex-rs/tui/src/exec_cell/render.rs index a3cc8cacae..98d9d07256 100644 --- a/codex-rs/tui/src/exec_cell/render.rs +++ b/codex-rs/tui/src/exec_cell/render.rs @@ -48,10 +48,16 @@ pub(crate) fn new_active_exec_command( }) } +#[derive(Clone)] +pub(crate) struct OutputLines { + pub(crate) lines: Vec>, + pub(crate) omitted: Option, +} + pub(crate) fn output_lines( output: Option<&CommandOutput>, params: OutputLinesParams, -) -> Vec> { +) -> OutputLines { let OutputLinesParams { only_err, include_angle_pipe, @@ -63,9 +69,19 @@ pub(crate) fn output_lines( stderr, .. } = match output { - Some(output) if only_err && output.exit_code == 0 => return vec![], + Some(output) if only_err && output.exit_code == 0 => { + return OutputLines { + lines: Vec::new(), + omitted: None, + }; + } Some(output) => output, - None => return vec![], + None => { + return OutputLines { + lines: Vec::new(), + omitted: None, + }; + } }; let src = if *exit_code == 0 { stdout } else { stderr }; @@ -73,7 +89,7 @@ pub(crate) fn output_lines( let total = lines.len(); let limit = TOOL_CALL_MAX_LINES; - let mut out = Vec::new(); + let mut out: Vec> = Vec::new(); let head_end = total.min(limit); for (i, raw) in lines[..head_end].iter().enumerate() { @@ -93,6 +109,11 @@ pub(crate) fn output_lines( } let show_ellipsis = total > 2 * limit; + let omitted = if show_ellipsis { + Some(total - 2 * limit) + } else { + None + }; if show_ellipsis { let omitted = total - 2 * limit; out.push(format!("… +{omitted} lines").into()); @@ -114,7 +135,10 @@ pub(crate) fn output_lines( out.push(line); } - out + OutputLines { + lines: out, + omitted, + } } pub(crate) fn spinner(start_time: Option) -> Span<'static> { @@ -371,7 +395,7 @@ impl ExecCell { } if let Some(output) = call.output.as_ref() { - let raw_output_lines = output_lines( + let raw_output = output_lines( Some(output), OutputLinesParams { only_err: false, @@ -380,15 +404,18 @@ impl ExecCell { }, ); - if raw_output_lines.is_empty() { + if raw_output.lines.is_empty() { lines.extend(prefix_lines( vec![Line::from("(no output)".dim())], Span::from(layout.output_block.initial_prefix).dim(), Span::from(layout.output_block.subsequent_prefix), )); } else { - let trimmed_output = - Self::truncate_lines_middle(&raw_output_lines, layout.output_max_lines); + let trimmed_output = Self::truncate_lines_middle( + &raw_output.lines, + layout.output_max_lines, + raw_output.omitted, + ); let mut wrapped_output: Vec> = Vec::new(); let output_wrap_width = layout.output_block.wrap_width(width); @@ -427,7 +454,11 @@ impl ExecCell { out } - fn truncate_lines_middle(lines: &[Line<'static>], max: usize) -> Vec> { + fn truncate_lines_middle( + lines: &[Line<'static>], + max: usize, + omitted_hint: Option, + ) -> Vec> { if max == 0 { return Vec::new(); } @@ -435,7 +466,17 @@ impl ExecCell { return lines.to_vec(); } if max == 1 { - return vec![Self::ellipsis_line(lines.len())]; + // Carry forward any previously omitted count and add any + // additionally hidden content lines from this truncation. + let base = omitted_hint.unwrap_or(0); + // When an existing ellipsis is present, `lines` already includes + // that single representation line; exclude it from the count of + // additionally omitted content lines. + let extra = lines + .len() + .saturating_sub(usize::from(omitted_hint.is_some())); + let omitted = base + extra; + return vec![Self::ellipsis_line(omitted)]; } let head = (max - 1) / 2; @@ -446,8 +487,12 @@ impl ExecCell { out.extend(lines[..head].iter().cloned()); } - let omitted = lines.len().saturating_sub(head + tail); - out.push(Self::ellipsis_line(omitted)); + let base = omitted_hint.unwrap_or(0); + let additional = lines + .len() + .saturating_sub(head + tail) + .saturating_sub(usize::from(omitted_hint.is_some())); + out.push(Self::ellipsis_line(base + additional)); if tail > 0 { out.extend(lines[lines.len() - tail..].iter().cloned()); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index aa0ae35d92..2cc0c3fd9e 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -1148,7 +1148,7 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell { lines.push(Line::from("✘ Failed to apply patch".magenta().bold())); if !stderr.trim().is_empty() { - lines.extend(output_lines( + let output = output_lines( Some(&CommandOutput { exit_code: 1, stdout: String::new(), @@ -1160,7 +1160,8 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell { include_angle_pipe: true, include_prefix: true, }, - )); + ); + lines.extend(output.lines); } PlainHistoryCell { lines }