From bbf42f4e128a1f0b50ce061bb24a9a48a9615f95 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Fri, 22 Aug 2025 14:03:58 -0700 Subject: [PATCH 01/16] improve performance of 'cargo test -p codex-tui' (#2593) before: ``` $ time cargo test -p codex-tui -q [...] cargo test -p codex-tui -q 39.89s user 10.77s system 98% cpu 51.328 total ``` after: ``` $ time cargo test -p codex-tui -q [...] cargo test -p codex-tui -q 1.37s user 0.64s system 29% cpu 6.699 total ``` the major offenders were the textarea fuzz test and the custom_terminal doctests. (i think the doctests were being recompiled every time which made them extra slow?) --- codex-rs/tui/src/bottom_pane/mod.rs | 5 - codex-rs/tui/src/bottom_pane/textarea.rs | 4 +- codex-rs/tui/src/custom_terminal.rs | 145 -------------------- codex-rs/tui/src/status_indicator_widget.rs | 19 ++- 4 files changed, 16 insertions(+), 157 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 04f5d4b9bf..e24975f1c7 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -492,8 +492,6 @@ mod tests { assert!(pane.active_view.is_some(), "active view should be present"); // Render and ensure the top row includes the Working header instead of the composer. - // Give the animation thread a moment to tick. - std::thread::sleep(std::time::Duration::from_millis(120)); let area = Rect::new(0, 0, 40, 3); let mut buf = Buffer::empty(area); (&pane).render_ref(area, &mut buf); @@ -525,9 +523,6 @@ mod tests { // Begin a task: show initial status. pane.set_task_running(true); - // Allow some frames so the animation thread ticks. - std::thread::sleep(std::time::Duration::from_millis(120)); - // Render and confirm the line contains the "Working" header. let area = Rect::new(0, 0, 40, 3); let mut buf = Buffer::empty(area); diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 8379deb9f8..e2c83f55c5 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -1473,7 +1473,7 @@ mod tests { .timestamp() as u64; let mut rng = rand::rngs::StdRng::seed_from_u64(pst_today_seed); - for _case in 0..10_000 { + for _case in 0..500 { let mut ta = TextArea::new(); let mut state = TextAreaState::default(); // Track element payloads we insert. Payloads use characters '[' and ']' which @@ -1497,7 +1497,7 @@ mod tests { let mut width: u16 = rng.random_range(1..=12); let mut height: u16 = rng.random_range(1..=4); - for _step in 0..200 { + for _step in 0..60 { // Mostly stable width/height, occasionally change if rng.random_bool(0.1) { width = rng.random_range(1..=12); diff --git a/codex-rs/tui/src/custom_terminal.rs b/codex-rs/tui/src/custom_terminal.rs index ef642b1a17..407ab3297f 100644 --- a/codex-rs/tui/src/custom_terminal.rs +++ b/codex-rs/tui/src/custom_terminal.rs @@ -70,20 +70,6 @@ impl Frame<'_> { /// Usually the area argument is the size of the current frame or a sub-area of the current /// frame (which can be obtained using [`Layout`] to split the total area). /// - /// # Example - /// - /// ```rust - /// # use ratatui::{backend::TestBackend, Terminal}; - /// # let backend = TestBackend::new(5, 5); - /// # let mut terminal = Terminal::new(backend).unwrap(); - /// # let mut frame = terminal.get_frame(); - /// use ratatui::{layout::Rect, widgets::Block}; - /// - /// let block = Block::new(); - /// let area = Rect::new(0, 0, 5, 5); - /// frame.render_widget(block, area); - /// ``` - /// /// [`Layout`]: crate::layout::Layout pub fn render_widget(&mut self, widget: W, area: Rect) { widget.render(area, self.buffer); @@ -93,22 +79,6 @@ impl Frame<'_> { /// /// Usually the area argument is the size of the current frame or a sub-area of the current /// frame (which can be obtained using [`Layout`] to split the total area). - /// - /// # Example - /// - /// ```rust - /// # #[cfg(feature = "unstable-widget-ref")] { - /// # use ratatui::{backend::TestBackend, Terminal}; - /// # let backend = TestBackend::new(5, 5); - /// # let mut terminal = Terminal::new(backend).unwrap(); - /// # let mut frame = terminal.get_frame(); - /// use ratatui::{layout::Rect, widgets::Block}; - /// - /// let block = Block::new(); - /// let area = Rect::new(0, 0, 5, 5); - /// frame.render_widget_ref(block, area); - /// # } - /// ``` #[allow(clippy::needless_pass_by_value)] pub fn render_widget_ref(&mut self, widget: W, area: Rect) { widget.render_ref(area, self.buffer); @@ -122,24 +92,6 @@ impl Frame<'_> { /// The last argument should be an instance of the [`StatefulWidget::State`] associated to the /// given [`StatefulWidget`]. /// - /// # Example - /// - /// ```rust - /// # use ratatui::{backend::TestBackend, Terminal}; - /// # let backend = TestBackend::new(5, 5); - /// # let mut terminal = Terminal::new(backend).unwrap(); - /// # let mut frame = terminal.get_frame(); - /// use ratatui::{ - /// layout::Rect, - /// widgets::{List, ListItem, ListState}, - /// }; - /// - /// let mut state = ListState::default().with_selected(Some(1)); - /// let list = List::new(vec![ListItem::new("Item 1"), ListItem::new("Item 2")]); - /// let area = Rect::new(0, 0, 5, 5); - /// frame.render_stateful_widget(list, area, &mut state); - /// ``` - /// /// [`Layout`]: crate::layout::Layout pub fn render_stateful_widget(&mut self, widget: W, area: Rect, state: &mut W::State) where @@ -156,26 +108,6 @@ impl Frame<'_> { /// /// The last argument should be an instance of the [`StatefulWidgetRef::State`] associated to /// the given [`StatefulWidgetRef`]. - /// - /// # Example - /// - /// ```rust - /// # #[cfg(feature = "unstable-widget-ref")] { - /// # use ratatui::{backend::TestBackend, Terminal}; - /// # let backend = TestBackend::new(5, 5); - /// # let mut terminal = Terminal::new(backend).unwrap(); - /// # let mut frame = terminal.get_frame(); - /// use ratatui::{ - /// layout::Rect, - /// widgets::{List, ListItem, ListState}, - /// }; - /// - /// let mut state = ListState::default().with_selected(Some(1)); - /// let list = List::new(vec![ListItem::new("Item 1"), ListItem::new("Item 2")]); - /// let area = Rect::new(0, 0, 5, 5); - /// frame.render_stateful_widget_ref(list, area, &mut state); - /// # } - /// ``` #[allow(clippy::needless_pass_by_value)] pub fn render_stateful_widget_ref(&mut self, widget: W, area: Rect, state: &mut W::State) where @@ -216,17 +148,6 @@ impl Frame<'_> { /// This count is particularly useful when dealing with dynamic content or animations where the /// state of the display changes over time. By tracking the frame count, developers can /// synchronize updates or changes to the content with the rendering process. - /// - /// # Examples - /// - /// ```rust - /// # use ratatui::{backend::TestBackend, Terminal}; - /// # let backend = TestBackend::new(5, 5); - /// # let mut terminal = Terminal::new(backend).unwrap(); - /// # let mut frame = terminal.get_frame(); - /// let current_count = frame.count(); - /// println!("Current frame count: {}", current_count); - /// ``` pub const fn count(&self) -> usize { self.count } @@ -277,19 +198,6 @@ where B: Backend, { /// Creates a new [`Terminal`] with the given [`Backend`] and [`TerminalOptions`]. - /// - /// # Example - /// - /// ```rust - /// use std::io::stdout; - /// - /// use ratatui::{backend::CrosstermBackend, layout::Rect, Terminal, TerminalOptions, Viewport}; - /// - /// let backend = CrosstermBackend::new(stdout()); - /// let viewport = Viewport::Fixed(Rect::new(0, 0, 10, 10)); - /// let terminal = Terminal::with_options(backend, TerminalOptions { viewport })?; - /// # std::io::Result::Ok(()) - /// ``` pub fn with_options(mut backend: B) -> io::Result { let screen_size = backend.size()?; let cursor_pos = backend.get_cursor_position()?; @@ -394,29 +302,6 @@ where /// previous frame to determine what has changed, and only the changes are written to the /// terminal. If the render callback does not fully render the frame, the terminal will not be /// in a consistent state. - /// - /// # Examples - /// - /// ``` - /// # let backend = ratatui::backend::TestBackend::new(10, 10); - /// # let mut terminal = ratatui::Terminal::new(backend)?; - /// use ratatui::{layout::Position, widgets::Paragraph}; - /// - /// // with a closure - /// terminal.draw(|frame| { - /// let area = frame.area(); - /// frame.render_widget(Paragraph::new("Hello World!"), area); - /// frame.set_cursor_position(Position { x: 0, y: 0 }); - /// })?; - /// - /// // or with a function - /// terminal.draw(render)?; - /// - /// fn render(frame: &mut ratatui::Frame) { - /// frame.render_widget(Paragraph::new("Hello World!"), frame.area()); - /// } - /// # std::io::Result::Ok(()) - /// ``` pub fn draw(&mut self, render_callback: F) -> io::Result<()> where F: FnOnce(&mut Frame), @@ -462,36 +347,6 @@ where /// previous frame to determine what has changed, and only the changes are written to the /// terminal. If the render function does not fully render the frame, the terminal will not be /// in a consistent state. - /// - /// # Examples - /// - /// ```should_panic - /// # use ratatui::layout::Position;; - /// # let backend = ratatui::backend::TestBackend::new(10, 10); - /// # let mut terminal = ratatui::Terminal::new(backend)?; - /// use std::io; - /// - /// use ratatui::widgets::Paragraph; - /// - /// // with a closure - /// terminal.try_draw(|frame| { - /// let value: u8 = "not a number".parse().map_err(io::Error::other)?; - /// let area = frame.area(); - /// frame.render_widget(Paragraph::new("Hello World!"), area); - /// frame.set_cursor_position(Position { x: 0, y: 0 }); - /// io::Result::Ok(()) - /// })?; - /// - /// // or with a function - /// terminal.try_draw(render)?; - /// - /// fn render(frame: &mut ratatui::Frame) -> io::Result<()> { - /// let value: u8 = "not a number".parse().map_err(io::Error::other)?; - /// frame.render_widget(Paragraph::new("Hello World!"), frame.area()); - /// Ok(()) - /// } - /// # io::Result::Ok(()) - /// ``` pub fn try_draw(&mut self, render_callback: F) -> io::Result<()> where F: FnOnce(&mut Frame) -> Result<(), E>, diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index df4910fe91..dbdd230da7 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -146,6 +146,15 @@ impl StatusIndicatorWidget { let since_start = self.start_time.elapsed(); (since_start.as_millis() / 100) as usize } + + /// Test-only helper to fast-forward the internal clock so animations + /// advance without sleeping. + #[cfg(test)] + pub(crate) fn test_fast_forward_frames(&mut self, frames: usize) { + let advance_ms = (frames as u64).saturating_mul(100); + // Move the start time into the past so `current_frame()` advances. + self.start_time = std::time::Instant::now() - std::time::Duration::from_millis(advance_ms); + } } impl WidgetRef for StatusIndicatorWidget { @@ -236,8 +245,8 @@ mod tests { w.restart_with_text("Hello".to_string()); let area = ratatui::layout::Rect::new(0, 0, 30, 1); - // Allow a short delay so the typewriter reveals the first character. - std::thread::sleep(std::time::Duration::from_millis(120)); + // Advance animation without sleeping. + w.test_fast_forward_frames(2); let mut buf = ratatui::buffer::Buffer::empty(area); w.render_ref(area, &mut buf); @@ -252,8 +261,8 @@ mod tests { let tx = AppEventSender::new(tx_raw); let mut w = StatusIndicatorWidget::new(tx, crate::tui::FrameRequester::test_dummy()); w.restart_with_text("Hi".to_string()); - // Ensure some frames elapse so we get a stable state. - std::thread::sleep(std::time::Duration::from_millis(120)); + // Advance animation without sleeping. + w.test_fast_forward_frames(2); let area = ratatui::layout::Rect::new(0, 0, 30, 1); let mut buf = ratatui::buffer::Buffer::empty(area); @@ -273,7 +282,7 @@ mod tests { let tx = AppEventSender::new(tx_raw); let mut w = StatusIndicatorWidget::new(tx, crate::tui::FrameRequester::test_dummy()); w.restart_with_text("Hello".to_string()); - std::thread::sleep(std::time::Duration::from_millis(120)); + w.test_fast_forward_frames(2); let area = ratatui::layout::Rect::new(0, 0, 30, 1); let mut buf = ratatui::buffer::Buffer::empty(area); From 57c498159ab92e83306ca21095f154b8a58aedb3 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Fri, 22 Aug 2025 14:04:21 -0700 Subject: [PATCH 02/16] test: simplify tests in config.rs (#2586) this is much easier to read, thanks @bolinfest for the suggestion. --- codex-rs/core/src/config.rs | 88 ++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 51 deletions(-) diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 2cde43ae08..67a54eb1d7 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -279,12 +279,14 @@ pub fn set_project_trusted(codex_home: &Path, project_path: &Path) -> anyhow::Re // Ensure top-level `projects` exists as a non-inline, explicit table. If it // exists but was previously represented as a non-table (e.g., inline), // replace it with an explicit table. + let mut created_projects_table = false; { let root = doc.as_table_mut(); let needs_table = !root.contains_key("projects") || root.get("projects").and_then(|i| i.as_table()).is_none(); if needs_table { root.insert("projects", toml_edit::table()); + created_projects_table = true; } } let Some(projects_tbl) = doc["projects"].as_table_mut() else { @@ -293,6 +295,12 @@ pub fn set_project_trusted(codex_home: &Path, project_path: &Path) -> anyhow::Re )); }; + // If we created the `projects` table ourselves, keep it implicit so we + // don't render a standalone `[projects]` header. + if created_projects_table { + projects_tbl.set_implicit(true); + } + // Ensure the per-project entry is its own explicit table. If it exists but // is not a table (e.g., an inline table), replace it with an explicit table. let needs_proj_table = !projects_tbl.contains_key(project_key.as_str()) @@ -1254,37 +1262,22 @@ disable_response_storage = true // Call the function under test set_project_trusted(codex_home.path(), project_dir.path())?; - // Read back the generated config.toml + // Read back the generated config.toml and assert exact contents let config_path = codex_home.path().join(CONFIG_TOML_FILE); let contents = std::fs::read_to_string(&config_path)?; - // Verify it does not use inline tables for the project entry - assert!( - !contents.contains("{ trust_level"), - "config.toml should not use inline tables:\n{}", - contents - ); - - // Verify the explicit table for the project exists. toml_edit may choose - // either basic (double-quoted) or literal (single-quoted) strings for keys - // containing backslashes (e.g., on Windows). Accept both forms. - let path_str = project_dir.path().to_string_lossy(); - let project_key_double = format!("[projects.\"{}\"]", path_str); - let project_key_single = format!("[projects.'{}']", path_str); - assert!( - contents.contains(&project_key_double) || contents.contains(&project_key_single), - "missing explicit project table header: expected to find `{}` or `{}` in:\n{}", - project_key_double, - project_key_single, - contents - ); - - // Verify the trust_level entry - assert!( - contents.contains("trust_level = \"trusted\""), - "missing trust_level entry in:\n{}", - contents + let raw_path = project_dir.path().to_string_lossy(); + let path_str = if raw_path.contains('\\') { + format!("'{}'", raw_path) + } else { + format!("\"{}\"", raw_path) + }; + let expected = format!( + r#"[projects.{path_str}] +trust_level = "trusted" +"# ); + assert_eq!(contents, expected); Ok(()) } @@ -1296,11 +1289,17 @@ disable_response_storage = true // Seed config.toml with an inline project entry under [projects] let config_path = codex_home.path().join(CONFIG_TOML_FILE); - let path_str = project_dir.path().to_string_lossy(); - // Use a literal-quoted key so Windows backslashes don't require escaping + let raw_path = project_dir.path().to_string_lossy(); + let path_str = if raw_path.contains('\\') { + format!("'{}'", raw_path) + } else { + format!("\"{}\"", raw_path) + }; + // Use a quoted key so backslashes don't require escaping on Windows let initial = format!( - "[projects]\n'{}' = {{ trust_level = \"untrusted\" }}\n", - path_str + r#"[projects] +{path_str} = {{ trust_level = "untrusted" }} +"# ); std::fs::create_dir_all(codex_home.path())?; std::fs::write(&config_path, initial)?; @@ -1310,28 +1309,15 @@ disable_response_storage = true let contents = std::fs::read_to_string(&config_path)?; - // Should not contain inline table representation anymore (accept both quote styles) - let inline_double = format!("\"{}\" = {{ trust_level = \"trusted\" }}", path_str); - let inline_single = format!("'{}' = {{ trust_level = \"trusted\" }}", path_str); - assert!( - !contents.contains(&inline_double) && !contents.contains(&inline_single), - "config.toml should not contain inline project table anymore:\n{}", - contents - ); + // Assert exact output after conversion to explicit table + let expected = format!( + r#"[projects] - // And explicit child table header for the project - let project_key_double = format!("[projects.\"{}\"]", path_str); - let project_key_single = format!("[projects.'{}']", path_str); - assert!( - contents.contains(&project_key_double) || contents.contains(&project_key_single), - "missing explicit project table header: expected to find `{}` or `{}` in:\n{}", - project_key_double, - project_key_single, - contents +[projects.{path_str}] +trust_level = "trusted" +"# ); - - // And the trust level value - assert!(contents.contains("trust_level = \"trusted\"")); + assert_eq!(contents, expected); Ok(()) } From 8ba80895927b08843329aed458a88c9f4b95e44d Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 22 Aug 2025 14:10:18 -0700 Subject: [PATCH 03/16] fix: prefer sending MCP structuredContent as the function call response, if available (#2594) Prior to this change, when we got a `CallToolResult` from an MCP server, we JSON-serialized its `content` field as the `content` to send back to the model as part of the function call output that we send back to the model. This meant that we were dropping the `structuredContent` on the floor. Though reading https://modelcontextprotocol.io/specification/2025-06-18/schema#tool, it appears that if `outputSchema` is specified, then `structuredContent` should be set, which seems to be a "higher-fidelity" response to the function call. This PR updates our handling of `CallToolResult` to prefer using the JSON-serialization of `structuredContent`, if present, using `content` as a fallback. Also, it appears that the sense of `success` was inverted prior to this PR! --- codex-rs/core/src/codex.rs | 152 +++++++++++++++++++++++++++++++++---- 1 file changed, 139 insertions(+), 13 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 4188a5df0b..e99d5fb64c 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1423,24 +1423,21 @@ async fn run_task( Some(ResponseInputItem::McpToolCallOutput { call_id, result }), ) => { items_to_record_in_conversation_history.push(item); - let (content, success): (String, Option) = match result { - Ok(CallToolResult { - content, - is_error, - structured_content: _, - }) => match serde_json::to_string(content) { - Ok(content) => (content, *is_error), - Err(e) => { - warn!("Failed to serialize MCP tool call output: {e}"); - (e.to_string(), Some(true)) - } + let output = match result { + Ok(call_tool_result) => { + convert_call_tool_result_to_function_call_output_payload( + call_tool_result, + ) + } + Err(err) => FunctionCallOutputPayload { + content: err.clone(), + success: Some(false), }, - Err(e) => (e.clone(), Some(true)), }; items_to_record_in_conversation_history.push( ResponseItem::FunctionCallOutput { call_id: call_id.clone(), - output: FunctionCallOutputPayload { content, success }, + output, }, ); } @@ -2639,3 +2636,132 @@ async fn drain_to_completed( } } } + +fn convert_call_tool_result_to_function_call_output_payload( + call_tool_result: &CallToolResult, +) -> FunctionCallOutputPayload { + let CallToolResult { + content, + is_error, + structured_content, + } = call_tool_result; + + // In terms of what to send back to the model, we prefer structured_content, + // if available, and fallback to content, otherwise. + let mut is_success = is_error != &Some(true); + let content = if let Some(structured_content) = structured_content + && structured_content != &serde_json::Value::Null + && let Ok(serialized_structured_content) = serde_json::to_string(&structured_content) + { + serialized_structured_content + } else { + match serde_json::to_string(&content) { + Ok(serialized_content) => serialized_content, + Err(err) => { + // If we could not serialize either content or structured_content to + // JSON, flag this as an error. + is_success = false; + err.to_string() + } + } + }; + + FunctionCallOutputPayload { + content, + success: Some(is_success), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use mcp_types::ContentBlock; + use mcp_types::TextContent; + use pretty_assertions::assert_eq; + use serde_json::json; + + fn text_block(s: &str) -> ContentBlock { + ContentBlock::TextContent(TextContent { + annotations: None, + text: s.to_string(), + r#type: "text".to_string(), + }) + } + + #[test] + fn prefers_structured_content_when_present() { + let ctr = CallToolResult { + // Content present but should be ignored because structured_content is set. + content: vec![text_block("ignored")], + is_error: None, + structured_content: Some(json!({ + "ok": true, + "value": 42 + })), + }; + + let got = convert_call_tool_result_to_function_call_output_payload(&ctr); + let expected = FunctionCallOutputPayload { + content: serde_json::to_string(&json!({ + "ok": true, + "value": 42 + })) + .unwrap(), + success: Some(true), + }; + + assert_eq!(expected, got); + } + + #[test] + fn falls_back_to_content_when_structured_is_null() { + let ctr = CallToolResult { + content: vec![text_block("hello"), text_block("world")], + is_error: None, + structured_content: Some(serde_json::Value::Null), + }; + + let got = convert_call_tool_result_to_function_call_output_payload(&ctr); + let expected = FunctionCallOutputPayload { + content: serde_json::to_string(&vec![text_block("hello"), text_block("world")]) + .unwrap(), + success: Some(true), + }; + + assert_eq!(expected, got); + } + + #[test] + fn success_flag_reflects_is_error_true() { + let ctr = CallToolResult { + content: vec![text_block("unused")], + is_error: Some(true), + structured_content: Some(json!({ "message": "bad" })), + }; + + let got = convert_call_tool_result_to_function_call_output_payload(&ctr); + let expected = FunctionCallOutputPayload { + content: serde_json::to_string(&json!({ "message": "bad" })).unwrap(), + success: Some(false), + }; + + assert_eq!(expected, got); + } + + #[test] + fn success_flag_true_with_no_error_and_content_used() { + let ctr = CallToolResult { + content: vec![text_block("alpha")], + is_error: Some(false), + structured_content: None, + }; + + let got = convert_call_tool_result_to_function_call_output_payload(&ctr); + let expected = FunctionCallOutputPayload { + content: serde_json::to_string(&vec![text_block("alpha")]).unwrap(), + success: Some(true), + }; + + assert_eq!(expected, got); + } +} From 097782c7755f5df3016f580f9c9bc9c7f9d47755 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Fri, 22 Aug 2025 15:18:54 -0700 Subject: [PATCH 04/16] Move models.rs to protocol (#2595) Moving models.rs to protocol so we can use them in `Codex` operations --- codex-rs/Cargo.lock | 3 ++ codex-rs/core/src/apply_patch.rs | 4 +-- codex-rs/core/src/chat_completions.rs | 40 +++++++++++++---------- codex-rs/core/src/client.rs | 2 +- codex-rs/core/src/client_common.rs | 4 +-- codex-rs/core/src/codex.rs | 16 ++++----- codex-rs/core/src/conversation_history.rs | 20 ++++++------ codex-rs/core/src/environment_context.rs | 4 +-- codex-rs/core/src/lib.rs | 1 - codex-rs/core/src/mcp_tool_call.rs | 4 +-- codex-rs/core/src/plan_tool.rs | 4 +-- codex-rs/core/src/rollout.rs | 2 +- codex-rs/protocol/Cargo.toml | 3 ++ codex-rs/protocol/src/lib.rs | 1 + codex-rs/{core => protocol}/src/models.rs | 1 - 15 files changed, 59 insertions(+), 50 deletions(-) rename codex-rs/{core => protocol}/src/models.rs (99%) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 2fdf9873e2..325b8ee0d5 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -923,13 +923,16 @@ dependencies = [ name = "codex-protocol" version = "0.0.0" dependencies = [ + "base64 0.22.1", "mcp-types", + "mime_guess", "pretty_assertions", "serde", "serde_bytes", "serde_json", "strum 0.27.2", "strum_macros 0.27.2", + "tracing", "ts-rs", "uuid", ] diff --git a/codex-rs/core/src/apply_patch.rs b/codex-rs/core/src/apply_patch.rs index 4f9292b6d7..56fd01feb1 100644 --- a/codex-rs/core/src/apply_patch.rs +++ b/codex-rs/core/src/apply_patch.rs @@ -1,13 +1,13 @@ use crate::codex::Session; use crate::codex::TurnContext; -use crate::models::FunctionCallOutputPayload; -use crate::models::ResponseInputItem; use crate::protocol::FileChange; use crate::protocol::ReviewDecision; use crate::safety::SafetyCheck; use crate::safety::assess_patch_safety; use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::ApplyPatchFileChange; +use codex_protocol::models::FunctionCallOutputPayload; +use codex_protocol::models::ResponseInputItem; use std::collections::HashMap; use std::path::PathBuf; diff --git a/codex-rs/core/src/chat_completions.rs b/codex-rs/core/src/chat_completions.rs index 7131b8f466..55b200911a 100644 --- a/codex-rs/core/src/chat_completions.rs +++ b/codex-rs/core/src/chat_completions.rs @@ -22,11 +22,11 @@ use crate::client_common::ResponseStream; use crate::error::CodexErr; use crate::error::Result; use crate::model_family::ModelFamily; -use crate::models::ContentItem; -use crate::models::ReasoningItemContent; -use crate::models::ResponseItem; use crate::openai_tools::create_tools_json_for_chat_completions_api; use crate::util::backoff; +use codex_protocol::models::ContentItem; +use codex_protocol::models::ReasoningItemContent; +use codex_protocol::models::ResponseItem; /// Implementation for the classic Chat Completions API. pub(crate) async fn stream_chat_completions( @@ -509,16 +509,19 @@ where // do NOT emit yet. Forward any other item (e.g. FunctionCall) right // away so downstream consumers see it. - let is_assistant_delta = matches!(&item, crate::models::ResponseItem::Message { role, .. } if role == "assistant"); + let is_assistant_delta = matches!(&item, codex_protocol::models::ResponseItem::Message { role, .. } if role == "assistant"); if is_assistant_delta { // Only use the final assistant message if we have not // seen any deltas; otherwise, deltas already built the // cumulative text and this would duplicate it. if this.cumulative.is_empty() - && let crate::models::ResponseItem::Message { content, .. } = &item + && let codex_protocol::models::ResponseItem::Message { content, .. } = + &item && let Some(text) = content.iter().find_map(|c| match c { - crate::models::ContentItem::OutputText { text } => Some(text), + codex_protocol::models::ContentItem::OutputText { text } => { + Some(text) + } _ => None, }) { @@ -542,26 +545,27 @@ where if !this.cumulative_reasoning.is_empty() && matches!(this.mode, AggregateMode::AggregatedOnly) { - let aggregated_reasoning = crate::models::ResponseItem::Reasoning { - id: String::new(), - summary: Vec::new(), - content: Some(vec![ - crate::models::ReasoningItemContent::ReasoningText { - text: std::mem::take(&mut this.cumulative_reasoning), - }, - ]), - encrypted_content: None, - }; + let aggregated_reasoning = + codex_protocol::models::ResponseItem::Reasoning { + id: String::new(), + summary: Vec::new(), + content: Some(vec![ + codex_protocol::models::ReasoningItemContent::ReasoningText { + text: std::mem::take(&mut this.cumulative_reasoning), + }, + ]), + encrypted_content: None, + }; this.pending .push_back(ResponseEvent::OutputItemDone(aggregated_reasoning)); emitted_any = true; } if !this.cumulative.is_empty() { - let aggregated_message = crate::models::ResponseItem::Message { + let aggregated_message = codex_protocol::models::ResponseItem::Message { id: None, role: "assistant".to_string(), - content: vec![crate::models::ContentItem::OutputText { + content: vec![codex_protocol::models::ContentItem::OutputText { text: std::mem::take(&mut this.cumulative), }], }; diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 43f3f1de9f..6206b4264a 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -37,13 +37,13 @@ use crate::flags::CODEX_RS_SSE_FIXTURE; use crate::model_family::ModelFamily; use crate::model_provider_info::ModelProviderInfo; use crate::model_provider_info::WireApi; -use crate::models::ResponseItem; use crate::openai_tools::create_tools_json_for_responses_api; use crate::protocol::TokenUsage; use crate::user_agent::get_codex_user_agent; use crate::util::backoff; use codex_protocol::config_types::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; +use codex_protocol::models::ResponseItem; use std::sync::Arc; #[derive(Debug, Deserialize)] diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index c9a8a42903..c320d8d0d7 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -1,13 +1,13 @@ use crate::config_types::Verbosity as VerbosityConfig; use crate::error::Result; use crate::model_family::ModelFamily; -use crate::models::ContentItem; -use crate::models::ResponseItem; use crate::openai_tools::OpenAiTool; use crate::protocol::TokenUsage; use codex_apply_patch::APPLY_PATCH_TOOL_INSTRUCTIONS; use codex_protocol::config_types::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; +use codex_protocol::models::ContentItem; +use codex_protocol::models::ResponseItem; use futures::Stream; use serde::Serialize; use std::borrow::Cow; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index e99d5fb64c..79b73a3335 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -56,14 +56,6 @@ use crate::exec_env::create_env; use crate::mcp_connection_manager::McpConnectionManager; use crate::mcp_tool_call::handle_mcp_tool_call; use crate::model_family::find_family_for_model; -use crate::models::ContentItem; -use crate::models::FunctionCallOutputPayload; -use crate::models::LocalShellAction; -use crate::models::ReasoningItemContent; -use crate::models::ReasoningItemReasoningSummary; -use crate::models::ResponseInputItem; -use crate::models::ResponseItem; -use crate::models::ShellToolCallParams; use crate::openai_tools::ApplyPatchToolArgs; use crate::openai_tools::ToolsConfig; use crate::openai_tools::get_openai_tools; @@ -108,6 +100,14 @@ use crate::user_notification::UserNotification; use crate::util::backoff; use codex_protocol::config_types::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; +use codex_protocol::models::ContentItem; +use codex_protocol::models::FunctionCallOutputPayload; +use codex_protocol::models::LocalShellAction; +use codex_protocol::models::ReasoningItemContent; +use codex_protocol::models::ReasoningItemReasoningSummary; +use codex_protocol::models::ResponseInputItem; +use codex_protocol::models::ResponseItem; +use codex_protocol::models::ShellToolCallParams; // A convenience extension trait for acquiring mutex locks where poisoning is // unrecoverable and should abort the program. This avoids scattered `.unwrap()` diff --git a/codex-rs/core/src/conversation_history.rs b/codex-rs/core/src/conversation_history.rs index 5955ce44b9..884c5a57df 100644 --- a/codex-rs/core/src/conversation_history.rs +++ b/codex-rs/core/src/conversation_history.rs @@ -1,4 +1,4 @@ -use crate::models::ResponseItem; +use codex_protocol::models::ResponseItem; /// Transcript of conversation history #[derive(Debug, Clone, Default)] @@ -66,7 +66,7 @@ impl ConversationHistory { self.items.push(ResponseItem::Message { id: None, role: "assistant".to_string(), - content: vec![crate::models::ContentItem::OutputText { + content: vec![codex_protocol::models::ContentItem::OutputText { text: delta.to_string(), }], }); @@ -120,11 +120,11 @@ fn is_api_message(message: &ResponseItem) -> bool { /// Helper to append the textual content from `src` into `dst` in place. fn append_text_content( - dst: &mut Vec, - src: &Vec, + dst: &mut Vec, + src: &Vec, ) { for c in src { - if let crate::models::ContentItem::OutputText { text } = c { + if let codex_protocol::models::ContentItem::OutputText { text } = c { append_text_delta(dst, text); } } @@ -132,15 +132,15 @@ fn append_text_content( /// Append a single text delta to the last OutputText item in `content`, or /// push a new OutputText item if none exists. -fn append_text_delta(content: &mut Vec, delta: &str) { - if let Some(crate::models::ContentItem::OutputText { text }) = content +fn append_text_delta(content: &mut Vec, delta: &str) { + if let Some(codex_protocol::models::ContentItem::OutputText { text }) = content .iter_mut() .rev() - .find(|c| matches!(c, crate::models::ContentItem::OutputText { .. })) + .find(|c| matches!(c, codex_protocol::models::ContentItem::OutputText { .. })) { text.push_str(delta); } else { - content.push(crate::models::ContentItem::OutputText { + content.push(codex_protocol::models::ContentItem::OutputText { text: delta.to_string(), }); } @@ -149,7 +149,7 @@ fn append_text_delta(content: &mut Vec, delta: &str) #[cfg(test)] mod tests { use super::*; - use crate::models::ContentItem; + use codex_protocol::models::ContentItem; fn assistant_msg(text: &str) -> ResponseItem { ResponseItem::Message { diff --git a/codex-rs/core/src/environment_context.rs b/codex-rs/core/src/environment_context.rs index c89d7ca6c7..1af4c9098a 100644 --- a/codex-rs/core/src/environment_context.rs +++ b/codex-rs/core/src/environment_context.rs @@ -2,12 +2,12 @@ use serde::Deserialize; use serde::Serialize; use strum_macros::Display as DeriveDisplay; -use crate::models::ContentItem; -use crate::models::ResponseItem; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; use crate::shell::Shell; use codex_protocol::config_types::SandboxMode; +use codex_protocol::models::ContentItem; +use codex_protocol::models::ResponseItem; use std::path::PathBuf; /// wraps environment context message in a tag for the model to parse more easily. diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 37f30a72d0..6d4699bceb 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -39,7 +39,6 @@ mod conversation_manager; pub use conversation_manager::ConversationManager; pub use conversation_manager::NewConversation; pub mod model_family; -mod models; mod openai_model_info; mod openai_tools; pub mod plan_tool; diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index e92d7e8481..57b29262d2 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -4,13 +4,13 @@ use std::time::Instant; use tracing::error; use crate::codex::Session; -use crate::models::FunctionCallOutputPayload; -use crate::models::ResponseInputItem; use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::McpInvocation; use crate::protocol::McpToolCallBeginEvent; use crate::protocol::McpToolCallEndEvent; +use codex_protocol::models::FunctionCallOutputPayload; +use codex_protocol::models::ResponseInputItem; /// Handles the specified tool call dispatches the appropriate /// `McpToolCallBegin` and `McpToolCallEnd` events to the `Session`. diff --git a/codex-rs/core/src/plan_tool.rs b/codex-rs/core/src/plan_tool.rs index bc39e4f6ce..a712b8b8dd 100644 --- a/codex-rs/core/src/plan_tool.rs +++ b/codex-rs/core/src/plan_tool.rs @@ -2,13 +2,13 @@ use std::collections::BTreeMap; use std::sync::LazyLock; use crate::codex::Session; -use crate::models::FunctionCallOutputPayload; -use crate::models::ResponseInputItem; use crate::openai_tools::JsonSchema; use crate::openai_tools::OpenAiTool; use crate::openai_tools::ResponsesApiTool; use crate::protocol::Event; use crate::protocol::EventMsg; +use codex_protocol::models::FunctionCallOutputPayload; +use codex_protocol::models::ResponseInputItem; // Use the canonical plan tool types from the protocol crate to ensure // type-identity matches events transported via `codex_protocol`. diff --git a/codex-rs/core/src/rollout.rs b/codex-rs/core/src/rollout.rs index f5a62ef8e6..46098c1637 100644 --- a/codex-rs/core/src/rollout.rs +++ b/codex-rs/core/src/rollout.rs @@ -22,7 +22,7 @@ use uuid::Uuid; use crate::config::Config; use crate::git_info::GitInfo; use crate::git_info::collect_git_info; -use crate::models::ResponseItem; +use codex_protocol::models::ResponseItem; const SESSIONS_SUBDIR: &str = "sessions"; diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index c94bdb8e1f..aac2444014 100644 --- a/codex-rs/protocol/Cargo.toml +++ b/codex-rs/protocol/Cargo.toml @@ -11,12 +11,15 @@ path = "src/lib.rs" workspace = true [dependencies] +base64 = "0.22.1" mcp-types = { path = "../mcp-types" } +mime_guess = "2.0.5" serde = { version = "1", features = ["derive"] } serde_bytes = "0.11" serde_json = "1" strum = "0.27.2" strum_macros = "0.27.2" +tracing = "0.1.41" ts-rs = { version = "11", features = ["uuid-impl", "serde-json-impl"] } uuid = { version = "1", features = ["serde", "v4"] } diff --git a/codex-rs/protocol/src/lib.rs b/codex-rs/protocol/src/lib.rs index 458ae6590c..d7aceeb9c8 100644 --- a/codex-rs/protocol/src/lib.rs +++ b/codex-rs/protocol/src/lib.rs @@ -1,6 +1,7 @@ pub mod config_types; pub mod mcp_protocol; pub mod message_history; +pub mod models; pub mod parse_command; pub mod plan_tool; pub mod protocol; diff --git a/codex-rs/core/src/models.rs b/codex-rs/protocol/src/models.rs similarity index 99% rename from codex-rs/core/src/models.rs rename to codex-rs/protocol/src/models.rs index aa092e1844..31e61d8be3 100644 --- a/codex-rs/core/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -204,7 +204,6 @@ impl From> for ResponseInputItem { None } }, - _ => None, }) .collect::>(), } From 34ac698bef7dd03c2ba202c64cc40c0f4c868e4c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 22 Aug 2025 15:45:14 -0700 Subject: [PATCH 05/16] chore(deps): bump serde_json from 1.0.142 to 1.0.143 in /codex-rs (#2498) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.142 to 1.0.143.
Release notes

Sourced from serde_json's releases.

v1.0.143

Commits
  • 10102c4 Release 1.0.143
  • 2a5b853 Replace super::super with absolute path within crate
  • 447170b Merge pull request 1271 from mickvangelderen/mick/impl-from-str-for-map
  • ec190d6 Merge pull request #1264 from xlambein/master
  • 8be6647 Merge pull request #1268 from SOF3/compact-default
  • ba5b3cc Revert "Pin nightly toolchain used for miri job"
  • fd35a02 Implement FromStr for Map<String, Value>
  • bea0fe6 Implement Default for CompactFormatter
  • 0c0e9f6 Add Clone and Debug impls to map iterators
  • See full diff in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=serde_json&package-manager=cargo&previous-version=1.0.142&new-version=1.0.143)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- codex-rs/Cargo.lock | 4 ++-- codex-rs/execpolicy/Cargo.toml | 2 +- codex-rs/file-search/Cargo.toml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 325b8ee0d5..72223c84bf 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4282,9 +4282,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.142" +version = "1.0.143" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "030fedb782600dcbd6f02d479bf0d817ac3bb40d644745b769d6a96bc3afc5a7" +checksum = "d401abef1d108fbd9cbaebc3e46611f4b1021f714a0597a71f41ee463f5f4a5a" dependencies = [ "indexmap 2.10.0", "itoa", diff --git a/codex-rs/execpolicy/Cargo.toml b/codex-rs/execpolicy/Cargo.toml index 9693d5c41f..e0e3d75e42 100644 --- a/codex-rs/execpolicy/Cargo.toml +++ b/codex-rs/execpolicy/Cargo.toml @@ -26,7 +26,7 @@ multimap = "0.10.0" path-absolutize = "3.1.1" regex-lite = "0.1" serde = { version = "1.0.194", features = ["derive"] } -serde_json = "1.0.142" +serde_json = "1.0.143" serde_with = { version = "3", features = ["macros"] } [dev-dependencies] diff --git a/codex-rs/file-search/Cargo.toml b/codex-rs/file-search/Cargo.toml index bf1e8e687f..f02d8402aa 100644 --- a/codex-rs/file-search/Cargo.toml +++ b/codex-rs/file-search/Cargo.toml @@ -17,5 +17,5 @@ clap = { version = "4", features = ["derive"] } ignore = "0.4.23" nucleo-matcher = "0.3.1" serde = { version = "1", features = ["derive"] } -serde_json = "1.0.142" +serde_json = "1.0.143" tokio = { version = "1", features = ["full"] } From 697c7cf4bfadd8d4a090c0c32bac07c88707009d Mon Sep 17 00:00:00 2001 From: Gabriel Peal Date: Fri, 22 Aug 2025 15:46:35 -0700 Subject: [PATCH 06/16] Fix flakiness in shell command approval test (#2547) ## Summary - read the shell exec approval request's actual id instead of assuming it is always 0 - use that id when validating and responding in the test ## Testing - `cargo test -p codex-mcp-server test_shell_command_approval_triggers_elicitation` ------ https://chatgpt.com/codex/tasks/task_i_68a6ab9c732c832c81522cbf11812be0 --- codex-rs/mcp-server/tests/codex_tool.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/codex-rs/mcp-server/tests/codex_tool.rs b/codex-rs/mcp-server/tests/codex_tool.rs index 1ebd10a77a..13866d970c 100644 --- a/codex-rs/mcp-server/tests/codex_tool.rs +++ b/codex-rs/mcp-server/tests/codex_tool.rs @@ -86,9 +86,7 @@ async fn shell_command_approval_triggers_elicitation() -> anyhow::Result<()> { ) .await??; - // This is the first request from the server, so the id should be 0 given - // how things are currently implemented. - let elicitation_request_id = RequestId::Integer(0); + let elicitation_request_id = elicitation_request.id.clone(); let params = serde_json::from_value::( elicitation_request .params From a5b2ebb49b2c0a8ca5a94adbdce2921572fdf56d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 22 Aug 2025 15:57:33 -0700 Subject: [PATCH 07/16] chore(deps): bump reqwest from 0.12.22 to 0.12.23 in /codex-rs (#2492) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [reqwest](https://github.com/seanmonstar/reqwest) from 0.12.22 to 0.12.23.
Release notes

Sourced from reqwest's releases.

v0.12.23

tl;dr

  • 🇺🇩🇸 Add ClientBuilder::unix_socket(path) option that will force all requests over that Unix Domain Socket.
  • 🔁 Add ClientBuilder::retries(policy) and reqwest::retry::Builder to configure automatic retries.
  • Add ClientBuilder::dns_resolver2() with more ergonomic argument bounds, allowing more resolver implementations.
  • Add http3_* options to blocking::ClientBuilder.
  • Fix default TCP timeout values to enabled and faster.
  • Fix SOCKS proxies to default to port 1080
  • (wasm) Add cache methods to RequestBuilder.

What's Changed

New Contributors

Full Changelog: https://github.com/seanmonstar/reqwest/compare/v0.12.22...v0.12.23

Changelog

Sourced from reqwest's changelog.

v0.12.23

  • Add ClientBuilder::unix_socket(path) option that will force all requests over that Unix Domain Socket.
  • Add ClientBuilder::retries(policy) and reqwest::retry::Builder to configure automatic retries.
  • Add ClientBuilder::dns_resolver2() with more ergonomic argument bounds, allowing more resolver implementations.
  • Add http3_* options to blocking::ClientBuilder.
  • Fix default TCP timeout values to enabled and faster.
  • Fix SOCKS proxies to default to port 1080
  • (wasm) Add cache methods to RequestBuilder.
Commits
  • ae7375b v0.12.23
  • 9aacdc1 feat: add dns_resolver2 that is more ergonomic and flexible (#2793)
  • 221be11 refactor: loosen retry for_host parameter bounds (#2792)
  • acd1b05 feat: add reqwest::retry policies (#2763)
  • 54b6022 feat: add ClientBuilder::unix_socket() option (#2624)
  • 6358cef fix: add default tcp keepalive and user_timeout values (#2780)
  • 21226a5 style(client): use std::task::ready! macro to simplify Poll branch matching...
  • 82086e7 feat: add request cache options for wasm (#2775)
  • 2a0f7a3 ci: use msrv-aware cargo in msrv job (#2779)
  • f186803 fix(proxy): restore default port 1080 for SOCKS proxies without explicit port...
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=reqwest&package-manager=cargo&previous-version=0.12.22&new-version=0.12.23)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- codex-rs/Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 72223c84bf..94702a83ed 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3888,9 +3888,9 @@ checksum = "ba39f3699c378cd8970968dcbff9c43159ea4cfbd88d43c00b22f2ef10a435d2" [[package]] name = "reqwest" -version = "0.12.22" +version = "0.12.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cbc931937e6ca3a06e3b6c0aa7841849b160a90351d6ab467a8b9b9959767531" +checksum = "d429f34c8092b2d42c7c93cec323bb4adeb7c67698f70839adec842ec10c7ceb" dependencies = [ "base64 0.22.1", "bytes", From 85099017fd1d43ba57031ee62aacdb207e58c8bd Mon Sep 17 00:00:00 2001 From: wkrettek <50168688+wkrettek@users.noreply.github.com> Date: Fri, 22 Aug 2025 16:05:39 -0700 Subject: [PATCH 08/16] Fix typo in AGENTS.md (#2518) - Change `examole` to `example` --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index eb2cacd50e..672c0086d6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,7 +2,7 @@ In the codex-rs folder where the rust code lives: -- Crate names are prefixed with `codex-`. For examole, the `core` folder's crate is named `codex-core` +- Crate names are prefixed with `codex-`. For example, the `core` folder's crate is named `codex-core` - When using format! and you can inline variables into {}, always do that. - Never add or modify any code related to `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` or `CODEX_SANDBOX_ENV_VAR`. - You operate in a sandbox where `CODEX_SANDBOX_NETWORK_DISABLED=1` will be set whenever you use the `shell` tool. Any existing code that uses `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` was authored with this fact in mind. It is often used to early exit out of tests that the author knew you would not be able to run given your sandbox limitations. From 6de9541f0a3c409eebf2c08bc7046b3d742e78fb Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Fri, 22 Aug 2025 16:06:41 -0700 Subject: [PATCH 09/16] tui: open transcript mode at the bottom (#2592) this got lost when we switched /diff to use the pager. --- codex-rs/tui/src/transcript_app.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codex-rs/tui/src/transcript_app.rs b/codex-rs/tui/src/transcript_app.rs index efb0c2de8b..44c7ea5cb4 100644 --- a/codex-rs/tui/src/transcript_app.rs +++ b/codex-rs/tui/src/transcript_app.rs @@ -28,7 +28,7 @@ impl TranscriptApp { pub(crate) fn new(transcript_lines: Vec>) -> Self { Self { transcript_lines, - scroll_offset: 0, + scroll_offset: usize::MAX, is_done: false, title: "T R A N S C R I P T".to_string(), } @@ -105,7 +105,7 @@ impl TranscriptApp { self.scroll_offset = self.scroll_offset.saturating_sub(area.height as usize); } KeyEvent { - code: KeyCode::PageDown, + code: KeyCode::PageDown | KeyCode::Char(' '), kind: KeyEventKind::Press | KeyEventKind::Repeat, .. } => { From d994019f3f2653c2027b2746b89661354b04cd38 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Fri, 22 Aug 2025 16:32:31 -0700 Subject: [PATCH 10/16] tui: coalesce command output; show unabridged commands in transcript (#2590) https://github.com/user-attachments/assets/effec7c7-732a-4b61-a2ae-3cb297b6b19b --- codex-rs/core/src/codex.rs | 43 ++- .../src/event_processor_with_human_output.rs | 1 + codex-rs/protocol/src/protocol.rs | 2 + codex-rs/tui/src/bottom_pane/mod.rs | 87 +++--- codex-rs/tui/src/chatwidget.rs | 27 +- codex-rs/tui/src/chatwidget/tests.rs | 115 +++++++- codex-rs/tui/src/exec_command.rs | 8 +- codex-rs/tui/src/history_cell.rs | 266 +++++++++++------- codex-rs/tui/src/render/line_utils.rs | 9 - codex-rs/tui/src/streaming/controller.rs | 12 - codex-rs/tui/src/streaming/mod.rs | 2 + codex-rs/tui/src/user_approval_widget.rs | 3 +- .../tests/fixtures/ideal-binary-response.txt | 28 +- 13 files changed, 394 insertions(+), 209 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 79b73a3335..cdf94210bd 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -701,7 +701,6 @@ impl Session { let _ = self.tx_event.send(event).await; } - #[allow(clippy::too_many_arguments)] async fn on_exec_command_end( &self, turn_diff_tracker: &mut TurnDiffTracker, @@ -721,6 +720,7 @@ impl Session { const MAX_STREAM_OUTPUT: usize = 5 * 1024; // 5KiB let stdout = stdout.text.chars().take(MAX_STREAM_OUTPUT).collect(); let stderr = stderr.text.chars().take(MAX_STREAM_OUTPUT).collect(); + let formatted_output = format_exec_output_str(output); let msg = if is_apply_patch { EventMsg::PatchApplyEnd(PatchApplyEndEvent { @@ -734,6 +734,7 @@ impl Session { call_id: call_id.to_string(), stdout, stderr, + formatted_output, duration: *duration, exit_code: *exit_code, }) @@ -2357,7 +2358,7 @@ async fn handle_container_exec_with_params( let ExecToolCallOutput { exit_code, .. } = &output; let is_success = *exit_code == 0; - let content = format_exec_output(output); + let content = format_exec_output(&output); ResponseInputItem::FunctionCallOutput { call_id: call_id.clone(), output: FunctionCallOutputPayload { @@ -2490,7 +2491,7 @@ async fn handle_sandbox_error( let ExecToolCallOutput { exit_code, .. } = &retry_output; let is_success = *exit_code == 0; - let content = format_exec_output(retry_output); + let content = format_exec_output(&retry_output); ResponseInputItem::FunctionCallOutput { call_id: call_id.clone(), @@ -2522,13 +2523,33 @@ async fn handle_sandbox_error( } } -/// Exec output is a pre-serialized JSON payload -fn format_exec_output(exec_output: ExecToolCallOutput) -> String { +fn format_exec_output_str(exec_output: &ExecToolCallOutput) -> String { let ExecToolCallOutput { exit_code, stdout, stderr, + .. + } = exec_output; + + let is_success = *exit_code == 0; + let output = if is_success { stdout } else { stderr }; + + let mut formatted_output = output.text.clone(); + if let Some(truncated_after_lines) = output.truncated_after_lines { + formatted_output.push_str(&format!( + "\n\n[Output truncated after {truncated_after_lines} lines: too many lines or bytes.]", + )); + } + + formatted_output +} + +/// Exec output is a pre-serialized JSON payload +fn format_exec_output(exec_output: &ExecToolCallOutput) -> String { + let ExecToolCallOutput { + exit_code, duration, + .. } = exec_output; #[derive(Serialize)] @@ -2546,20 +2567,12 @@ fn format_exec_output(exec_output: ExecToolCallOutput) -> String { // round to 1 decimal place let duration_seconds = ((duration.as_secs_f32()) * 10.0).round() / 10.0; - let is_success = exit_code == 0; - let output = if is_success { stdout } else { stderr }; - - let mut formatted_output = output.text; - if let Some(truncated_after_lines) = output.truncated_after_lines { - formatted_output.push_str(&format!( - "\n\n[Output truncated after {truncated_after_lines} lines: too many lines or bytes.]", - )); - } + let formatted_output = format_exec_output_str(exec_output); let payload = ExecOutput { output: &formatted_output, metadata: ExecMetadata { - exit_code, + exit_code: *exit_code, duration_seconds, }, }; diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 9a562cbd4d..1a42fd0c61 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -291,6 +291,7 @@ impl EventProcessor for EventProcessorWithHumanOutput { stderr, duration, exit_code, + .. }) => { let exec_command = self.call_id_to_command.remove(&call_id); let (duration, call) = if let Some(ExecCommandBegin { command, .. }) = exec_command diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index fbe052bf95..f337f94fe6 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -674,6 +674,8 @@ pub struct ExecCommandEndEvent { pub exit_code: i32, /// The duration of the command execution. pub duration: Duration, + /// Formatted output from the command, as seen by the model. + pub formatted_output: String, } #[derive(Debug, Clone, Deserialize, Serialize)] diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index e24975f1c7..02ea6cb62e 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -9,6 +9,8 @@ use codex_core::protocol::TokenUsage; use codex_file_search::FileMatch; use crossterm::event::KeyEvent; use ratatui::buffer::Buffer; +use ratatui::layout::Constraint; +use ratatui::layout::Layout; use ratatui::layout::Rect; use ratatui::widgets::WidgetRef; @@ -95,8 +97,31 @@ impl BottomPane { } else { self.composer.desired_height(width) }; + let top_pad = if self.active_view.is_none() || self.status_view_active { + 1 + } else { + 0 + }; + view_height + .saturating_add(Self::BOTTOM_PAD_LINES) + .saturating_add(top_pad) + } - view_height.saturating_add(Self::BOTTOM_PAD_LINES) + fn layout(&self, area: Rect) -> Rect { + let top = if self.active_view.is_none() || self.status_view_active { + 1 + } else { + 0 + }; + + let [_, content, _] = Layout::vertical([ + Constraint::Max(top), + Constraint::Min(1), + Constraint::Max(BottomPane::BOTTOM_PAD_LINES), + ]) + .areas(area); + + content } pub fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { @@ -104,10 +129,11 @@ impl BottomPane { // status indicator shown while a task is running, or approval modal). // In these states the textarea is not interactable, so we should not // show its caret. - if self.active_view.is_some() { + if self.active_view.is_some() || self.status_view_active { None } else { - self.composer.cursor_pos(area) + let content = self.layout(area); + self.composer.cursor_pos(content) } } @@ -365,31 +391,12 @@ impl BottomPane { impl WidgetRef for &BottomPane { fn render_ref(&self, area: Rect, buf: &mut Buffer) { + let content = self.layout(area); + if let Some(view) = &self.active_view { - // Reserve bottom padding lines; keep at least 1 line for the view. - let avail = area.height; - if avail > 0 { - let pad = BottomPane::BOTTOM_PAD_LINES.min(avail.saturating_sub(1)); - let view_rect = Rect { - x: area.x, - y: area.y, - width: area.width, - height: avail - pad, - }; - view.render(view_rect, buf); - } + view.render(content, buf); } else { - let avail = area.height; - if avail > 0 { - let composer_rect = Rect { - x: area.x, - y: area.y, - width: area.width, - // Reserve bottom padding - height: avail - BottomPane::BOTTOM_PAD_LINES.min(avail.saturating_sub(1)), - }; - (&self.composer).render_ref(composer_rect, buf); - } + (&self.composer).render_ref(content, buf); } } } @@ -495,13 +502,13 @@ mod tests { let area = Rect::new(0, 0, 40, 3); let mut buf = Buffer::empty(area); (&pane).render_ref(area, &mut buf); - let mut row0 = String::new(); + let mut row1 = String::new(); for x in 0..area.width { - row0.push(buf[(x, 0)].symbol().chars().next().unwrap_or(' ')); + row1.push(buf[(x, 1)].symbol().chars().next().unwrap_or(' ')); } assert!( - row0.contains("Working"), - "expected Working header after denial: {row0:?}" + row1.contains("Working"), + "expected Working header after denial on row 1: {row1:?}" ); // Drain the channel to avoid unused warnings. @@ -523,14 +530,13 @@ mod tests { // Begin a task: show initial status. pane.set_task_running(true); - // Render and confirm the line contains the "Working" header. let area = Rect::new(0, 0, 40, 3); let mut buf = Buffer::empty(area); (&pane).render_ref(area, &mut buf); let mut row0 = String::new(); for x in 0..area.width { - row0.push(buf[(x, 0)].symbol().chars().next().unwrap_or(' ')); + row0.push(buf[(x, 1)].symbol().chars().next().unwrap_or(' ')); } assert!( row0.contains("Working"), @@ -563,12 +569,12 @@ mod tests { let mut buf = Buffer::empty(area); (&pane).render_ref(area, &mut buf); - // Top row contains the status header + // Row 1 contains the status header (row 0 is the spacer) let mut top = String::new(); for x in 0..area.width { - top.push(buf[(x, 0)].symbol().chars().next().unwrap_or(' ')); + top.push(buf[(x, 1)].symbol().chars().next().unwrap_or(' ')); } - assert_eq!(buf[(0, 0)].symbol().chars().next().unwrap_or(' '), '▌'); + assert_eq!(buf[(0, 1)].symbol().chars().next().unwrap_or(' '), '▌'); assert!( top.contains("Working"), "expected Working header on top row: {top:?}" @@ -605,7 +611,7 @@ mod tests { pane.set_task_running(true); - // Height=2 → pad shrinks to 1; bottom row is blank, top row has spinner. + // Height=2 → with spacer, spinner on row 1; no bottom padding. let area2 = Rect::new(0, 0, 20, 2); let mut buf2 = Buffer::empty(area2); (&pane).render_ref(area2, &mut buf2); @@ -615,13 +621,10 @@ mod tests { row0.push(buf2[(x, 0)].symbol().chars().next().unwrap_or(' ')); row1.push(buf2[(x, 1)].symbol().chars().next().unwrap_or(' ')); } + assert!(row0.trim().is_empty(), "expected spacer on row 0: {row0:?}"); assert!( - row0.contains("Working"), - "expected Working header on row 0: {row0:?}" - ); - assert!( - row1.trim().is_empty(), - "expected bottom padding on row 1: {row1:?}" + row1.contains("Working"), + "expected Working on row 1: {row1:?}" ); // Height=1 → no padding; single row is the spinner. diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 0e4bd85664..e3097c9162 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -105,6 +105,7 @@ pub(crate) struct ChatWidget { full_reasoning_buffer: String, session_id: Option, frame_requester: FrameRequester, + last_history_was_exec: bool, } struct UserMessage { @@ -376,6 +377,9 @@ impl ChatWidget { self.bottom_pane.set_task_running(false); self.task_complete_pending = false; } + // A completed stream indicates non-exec content was just inserted. + // Reset the exec header grouping so the next exec shows its header. + self.last_history_was_exec = false; self.flush_interrupt_queue(); } } @@ -401,6 +405,7 @@ impl ChatWidget { exit_code: ev.exit_code, stdout: ev.stdout.clone(), stderr: ev.stderr.clone(), + formatted_output: ev.formatted_output.clone(), }, )); @@ -408,9 +413,16 @@ impl ChatWidget { self.active_exec_cell = None; let pending = std::mem::take(&mut self.pending_exec_completions); for (command, parsed, output) in pending { - self.add_to_history(history_cell::new_completed_exec_command( - command, parsed, output, - )); + let include_header = !self.last_history_was_exec; + let cell = history_cell::new_completed_exec_command( + command, + parsed, + output, + include_header, + ev.duration, + ); + self.add_to_history(cell); + self.last_history_was_exec = true; } } } @@ -473,9 +485,11 @@ impl ChatWidget { exec.parsed.extend(ev.parsed_cmd); } _ => { + let include_header = !self.last_history_was_exec; self.active_exec_cell = Some(history_cell::new_active_exec_command( ev.command, ev.parsed_cmd, + include_header, )); } } @@ -565,6 +579,7 @@ impl ChatWidget { reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), session_id: None, + last_history_was_exec: false, } } @@ -713,13 +728,19 @@ impl ChatWidget { fn flush_active_exec_cell(&mut self) { if let Some(active) = self.active_exec_cell.take() { + self.last_history_was_exec = true; self.app_event_tx .send(AppEvent::InsertHistoryCell(Box::new(active))); } } fn add_to_history(&mut self, cell: impl HistoryCell + 'static) { + // Only break exec grouping if the cell renders visible lines. + let has_display_lines = !cell.display_lines().is_empty(); self.flush_active_exec_cell(); + if has_display_lines { + self.last_history_was_exec = false; + } self.app_event_tx .send(AppEvent::InsertHistoryCell(Box::new(cell))); } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index e43dfc7824..04dd3ad389 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -44,6 +44,31 @@ fn test_config() -> Config { .expect("config") } +// Backward-compat shim for older session logs that predate the +// `formatted_output` field on ExecCommandEnd events. +fn upgrade_event_payload_for_tests(mut payload: serde_json::Value) -> serde_json::Value { + if let Some(obj) = payload.as_object_mut() + && let Some(msg) = obj.get_mut("msg") + && let Some(m) = msg.as_object_mut() + { + let ty = m.get("type").and_then(|v| v.as_str()).unwrap_or(""); + if ty == "exec_command_end" && !m.contains_key("formatted_output") { + let stdout = m.get("stdout").and_then(|v| v.as_str()).unwrap_or(""); + let stderr = m.get("stderr").and_then(|v| v.as_str()).unwrap_or(""); + let formatted = if stderr.is_empty() { + stdout.to_string() + } else { + format!("{stdout}{stderr}") + }; + m.insert( + "formatted_output".to_string(), + serde_json::Value::String(formatted), + ); + } + } + payload +} + #[test] fn final_answer_without_newline_is_flushed_immediately() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); @@ -157,6 +182,7 @@ fn make_chatwidget_manual() -> ( full_reasoning_buffer: String::new(), session_id: None, frame_requester: crate::tui::FrameRequester::test_dummy(), + last_history_was_exec: false, }; (widget, rx, op_rx) } @@ -239,6 +265,7 @@ fn exec_history_cell_shows_working_then_completed() { stderr: String::new(), exit_code: 0, duration: std::time::Duration::from_millis(5), + formatted_output: "done".into(), }), }); @@ -250,8 +277,12 @@ fn exec_history_cell_shows_working_then_completed() { ); let blob = lines_to_single_string(&cells[0]); assert!( - blob.contains("Completed"), - "expected completed exec cell to show Completed header: {blob:?}" + blob.contains('✓'), + "expected completed exec cell to show success marker: {blob:?}" + ); + assert!( + blob.contains("echo done"), + "expected command text to be present: {blob:?}" ); } @@ -284,6 +315,7 @@ fn exec_history_cell_shows_working_then_failed() { stderr: "error".into(), exit_code: 2, duration: std::time::Duration::from_millis(7), + formatted_output: "".into(), }), }); @@ -295,9 +327,80 @@ fn exec_history_cell_shows_working_then_failed() { ); let blob = lines_to_single_string(&cells[0]); assert!( - blob.contains("Failed (exit 2)"), - "expected completed exec cell to show Failed header with exit code: {blob:?}" + blob.contains('✗'), + "expected failure marker present: {blob:?}" ); + assert!( + blob.contains("false"), + "expected command text present: {blob:?}" + ); +} + +#[test] +fn exec_history_extends_previous_when_consecutive() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + // First command + chat.handle_codex_event(Event { + id: "call-a".into(), + msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { + call_id: "call-a".into(), + command: vec!["bash".into(), "-lc".into(), "echo one".into()], + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + parsed_cmd: vec![ + codex_core::parse_command::ParsedCommand::Unknown { + cmd: "echo one".into(), + } + .into(), + ], + }), + }); + chat.handle_codex_event(Event { + id: "call-a".into(), + msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { + call_id: "call-a".into(), + stdout: "one".into(), + stderr: String::new(), + exit_code: 0, + duration: std::time::Duration::from_millis(5), + formatted_output: "one".into(), + }), + }); + let first_cells = drain_insert_history(&mut rx); + assert_eq!(first_cells.len(), 1, "first exec should insert history"); + + // Second command + chat.handle_codex_event(Event { + id: "call-b".into(), + msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { + call_id: "call-b".into(), + command: vec!["bash".into(), "-lc".into(), "echo two".into()], + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + parsed_cmd: vec![ + codex_core::parse_command::ParsedCommand::Unknown { + cmd: "echo two".into(), + } + .into(), + ], + }), + }); + chat.handle_codex_event(Event { + id: "call-b".into(), + msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { + call_id: "call-b".into(), + stdout: "two".into(), + stderr: String::new(), + exit_code: 0, + duration: std::time::Duration::from_millis(5), + formatted_output: "two".into(), + }), + }); + let second_cells = drain_insert_history(&mut rx); + assert_eq!(second_cells.len(), 1, "second exec should extend history"); + let first_blob = lines_to_single_string(&first_cells[0]); + let second_blob = lines_to_single_string(&second_cells[0]); + assert!(first_blob.contains('✓')); + assert!(second_blob.contains("echo two")); } #[tokio::test(flavor = "current_thread")] @@ -340,7 +443,9 @@ async fn binary_size_transcript_matches_ideal_fixture() { match kind { "codex_event" => { if let Some(payload) = v.get("payload") { - let ev: Event = serde_json::from_value(payload.clone()).expect("parse"); + let ev: Event = + serde_json::from_value(upgrade_event_payload_for_tests(payload.clone())) + .expect("parse"); chat.handle_codex_event(ev); while let Ok(app_ev) = rx.try_recv() { match app_ev { diff --git a/codex-rs/tui/src/exec_command.rs b/codex-rs/tui/src/exec_command.rs index db3bf2fef4..1923fa3b69 100644 --- a/codex-rs/tui/src/exec_command.rs +++ b/codex-rs/tui/src/exec_command.rs @@ -9,13 +9,7 @@ pub(crate) fn escape_command(command: &[String]) -> String { pub(crate) fn strip_bash_lc_and_escape(command: &[String]) -> String { match command { - // exactly three items - [first, second, third] - // first two must be "bash", "-lc" - if first == "bash" && second == "-lc" => - { - third.clone() // borrow `third` - } + [first, second, third] if first == "bash" && second == "-lc" => third.clone(), _ => escape_command(command), } } diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 0cbb4888df..3e51ffd036 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -29,10 +29,10 @@ use ratatui::prelude::*; use ratatui::style::Color; use ratatui::style::Modifier; use ratatui::style::Style; +use ratatui::style::Stylize; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; -use shlex::try_join as shlex_try_join; use std::collections::HashMap; use std::io::Cursor; use std::path::PathBuf; @@ -46,6 +46,7 @@ pub(crate) struct CommandOutput { pub(crate) exit_code: i32, pub(crate) stdout: String, pub(crate) stderr: String, + pub(crate) formatted_output: String, } pub(crate) enum PatchEventType { @@ -104,6 +105,8 @@ pub(crate) struct ExecCell { pub(crate) parsed: Vec, pub(crate) output: Option, start_time: Option, + duration: Option, + include_header: bool, } impl HistoryCell for ExecCell { fn display_lines(&self) -> Vec> { @@ -112,15 +115,63 @@ impl HistoryCell for ExecCell { &self.parsed, self.output.as_ref(), self.start_time, + self.include_header, ) } + + fn transcript_lines(&self) -> Vec> { + let mut lines: Vec> = vec!["".into()]; + + let cmd_display = strip_bash_lc_and_escape(&self.command); + for (i, part) in cmd_display.lines().enumerate() { + if i == 0 { + lines.push(Line::from(vec!["$ ".magenta(), part.to_string().into()])); + } else { + lines.push(Line::from(vec![" ".into(), part.to_string().into()])); + } + } + + // Command output: include full stdout and stderr (no truncation) + if let Some(output) = self.output.as_ref() { + lines.extend(output.formatted_output.lines().map(ansi_escape_line)); + } + + if let Some(output) = self.output.as_ref() { + let duration = self + .duration + .map(format_duration) + .unwrap_or_else(|| "unknown".to_string()); + let mut result = if output.exit_code == 0 { + Line::from("✓".green().bold()) + } else { + Line::from(vec![ + "✗".red().bold(), + format!(" ({})", output.exit_code).into(), + ]) + }; + + result.push_span(format!(" • {duration}").dim()); + lines.push(result); + } + + lines + } } impl WidgetRef for &ExecCell { fn render_ref(&self, area: Rect, buf: &mut Buffer) { + if area.height == 0 { + return; + } + let content_area = Rect { + x: area.x, + y: area.y, + width: area.width, + height: area.height, + }; Paragraph::new(Text::from(self.display_lines())) .wrap(Wrap { trim: false }) - .render(area, buf); + .render(content_area, buf); } } @@ -131,8 +182,8 @@ struct CompletedMcpToolCallWithImageOutput { impl HistoryCell for CompletedMcpToolCallWithImageOutput { fn display_lines(&self) -> Vec> { vec![ - Line::from("tool result (image output omitted)"), Line::from(""), + Line::from("tool result (image output omitted)"), ] } } @@ -179,6 +230,7 @@ pub(crate) fn new_session_info( }; let lines: Vec> = vec![ + Line::from(Span::from("")), Line::from(vec![ Span::raw(">_ ").dim(), Span::styled( @@ -194,17 +246,16 @@ pub(crate) fn new_session_info( Line::from(format!(" /status - {}", SlashCommand::Status.description()).dim()), Line::from(format!(" /approvals - {}", SlashCommand::Approvals.description()).dim()), Line::from(format!(" /model - {}", SlashCommand::Model.description()).dim()), - Line::from("".dim()), ]; PlainHistoryCell { lines } } else if config.model == model { PlainHistoryCell { lines: Vec::new() } } else { let lines = vec![ + Line::from(""), Line::from("model changed:".magenta().bold()), Line::from(format!("requested: {}", config.model)), Line::from(format!("used: {model}")), - Line::from(""), ]; PlainHistoryCell { lines } } @@ -212,9 +263,9 @@ pub(crate) fn new_session_info( pub(crate) fn new_user_prompt(message: String) -> PlainHistoryCell { let mut lines: Vec> = Vec::new(); + lines.push(Line::from("")); lines.push(Line::from("user".cyan().bold())); lines.extend(message.lines().map(|l| Line::from(l.to_string()))); - lines.push(Line::from("")); PlainHistoryCell { lines } } @@ -222,12 +273,15 @@ pub(crate) fn new_user_prompt(message: String) -> PlainHistoryCell { pub(crate) fn new_active_exec_command( command: Vec, parsed: Vec, + include_header: bool, ) -> ExecCell { ExecCell { command, parsed, output: None, start_time: Some(Instant::now()), + duration: None, + include_header, } } @@ -235,76 +289,61 @@ pub(crate) fn new_completed_exec_command( command: Vec, parsed: Vec, output: CommandOutput, + include_header: bool, + duration: Duration, ) -> ExecCell { ExecCell { command, parsed, output: Some(output), start_time: None, + duration: Some(duration), + include_header, } } -fn exec_duration(start: Instant) -> String { - format!("{}s", start.elapsed().as_secs()) -} - fn exec_command_lines( command: &[String], parsed: &[ParsedCommand], output: Option<&CommandOutput>, start_time: Option, + include_header: bool, ) -> Vec> { match parsed.is_empty() { - true => new_exec_command_generic(command, output, start_time), - false => new_parsed_command(command, parsed, output, start_time), + true => new_exec_command_generic(command, output, start_time, include_header), + false => new_parsed_command(command, parsed, output, start_time, include_header), } } fn new_parsed_command( - command: &[String], + _command: &[String], parsed_commands: &[ParsedCommand], output: Option<&CommandOutput>, start_time: Option, + include_header: bool, ) -> Vec> { let mut lines: Vec = Vec::new(); - match output { - None => { - let mut spans = vec!["⚙︎ Working".magenta().bold()]; - if let Some(st) = start_time { - let dur = exec_duration(st); - spans.push(format!(" • {dur}").dim()); - } - lines.push(Line::from(spans)); - } - Some(o) if o.exit_code == 0 => { - lines.push(Line::from(vec!["✓".green(), " Completed".into()])); - } - Some(o) => { - lines.push(Line::from(vec![ - "✗".red(), - format!(" Failed (exit {})", o.exit_code).into(), - ])); - } - }; - - // Optionally include the complete, unaltered command from the model. - if std::env::var("SHOW_FULL_COMMANDS") - .map(|v| !v.is_empty()) - .unwrap_or(false) - { - let full_cmd = shlex_try_join(command.iter().map(|s| s.as_str())) - .unwrap_or_else(|_| command.join(" ")); - lines.push(Line::from(vec![ - Span::styled(" └ ", Style::default().add_modifier(Modifier::DIM)), - Span::styled( - full_cmd, - Style::default() - .add_modifier(Modifier::DIM) - .add_modifier(Modifier::ITALIC), - ), - ])); + // Leading spacer and header line above command list + if include_header { + lines.push(Line::from("")); + lines.push(Line::from(">_".magenta())); } - for (i, parsed) in parsed_commands.iter().enumerate() { + // Determine the leading status marker: spinner while running, ✓ on success, ✗ on failure. + let status_marker: Span<'static> = match output { + None => { + // Animated braille spinner – choose frame based on elapsed time. + const FRAMES: &[char] = &['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']; + let idx = start_time + .map(|st| ((st.elapsed().as_millis() / 100) as usize) % FRAMES.len()) + .unwrap_or(0); + let ch = FRAMES[idx]; + Span::raw(format!("{ch}")) + } + Some(o) if o.exit_code == 0 => Span::styled("✓", Style::default().fg(Color::Green)), + Some(_) => Span::styled("✗", Style::default().fg(Color::Red)), + }; + + for parsed in parsed_commands.iter() { let text = match parsed { ParsedCommand::Read { name, .. } => format!("📖 {name}"), ParsedCommand::ListFiles { cmd, path } => match path { @@ -323,19 +362,25 @@ fn new_parsed_command( ParsedCommand::Unknown { cmd } => format!("⌨️ {cmd}"), ParsedCommand::Noop { cmd } => format!("🔄 {cmd}"), }; - - let first_prefix = if i == 0 { " └ " } else { " " }; + // Prefix: two spaces, marker, space. Continuations align under the text block. for (j, line_text) in text.lines().enumerate() { - let prefix = if j == 0 { first_prefix } else { " " }; - lines.push(Line::from(vec![ - Span::styled(prefix, Style::default().add_modifier(Modifier::DIM)), - line_text.to_string().dim(), - ])); + if j == 0 { + lines.push(Line::from(vec![ + " ".into(), + status_marker.clone(), + " ".into(), + line_text.to_string().light_blue(), + ])); + } else { + lines.push(Line::from(vec![ + " ".into(), + line_text.to_string().light_blue(), + ])); + } } } lines.extend(output_lines(output, true, false)); - lines.push(Line::from("")); lines } @@ -344,29 +389,44 @@ fn new_exec_command_generic( command: &[String], output: Option<&CommandOutput>, start_time: Option, + include_header: bool, ) -> Vec> { let mut lines: Vec> = Vec::new(); - let command_escaped = strip_bash_lc_and_escape(command); - let mut cmd_lines = command_escaped.lines(); - if let Some(first) = cmd_lines.next() { - let mut spans: Vec = vec!["⚡ Running".magenta()]; - if let Some(st) = start_time { - let dur = exec_duration(st); - spans.push(format!(" • {dur}").dim()); - } - spans.push(" ".into()); - spans.push(first.to_string().into()); - lines.push(Line::from(spans)); - } else { - let mut spans: Vec = vec!["⚡ Running".magenta()]; - if let Some(st) = start_time { - let dur = exec_duration(st); - spans.push(format!(" • {dur}").dim()); - } - lines.push(Line::from(spans)); + // Leading spacer and header line above command list + if include_header { + lines.push(Line::from("")); + lines.push(Line::from(">_".magenta())); } - for cont in cmd_lines { - lines.push(Line::from(cont.to_string())); + let command_escaped = strip_bash_lc_and_escape(command); + + // Determine marker: spinner while running, ✓/✗ when completed + let status_marker: Span<'static> = match output { + None => { + const FRAMES: &[char] = &['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']; + let idx = start_time + .map(|st| ((st.elapsed().as_millis() / 100) as usize) % FRAMES.len()) + .unwrap_or(0); + let ch = FRAMES[idx]; + Span::raw(format!("{ch}")) + } + Some(o) if o.exit_code == 0 => Span::styled("✓", Style::default().fg(Color::Green)), + Some(_) => Span::styled("✗", Style::default().fg(Color::Red)), + }; + + for (i, line) in command_escaped.lines().enumerate() { + if i == 0 { + lines.push(Line::from(vec![ + Span::raw(" "), + status_marker.clone(), + Span::raw(" "), + Span::raw(line.to_string()), + ])); + } else { + lines.push(Line::from(vec![ + Span::styled(" ", Style::default().add_modifier(Modifier::DIM)), + Span::raw(line.to_string()), + ])); + } } lines.extend(output_lines(output, false, true)); @@ -377,9 +437,9 @@ fn new_exec_command_generic( pub(crate) fn new_active_mcp_tool_call(invocation: McpInvocation) -> PlainHistoryCell { let title_line = Line::from(vec!["tool".magenta(), " running...".dim()]); let lines: Vec = vec![ + Line::from(""), title_line, format_mcp_invocation(invocation.clone()), - Line::from(""), ]; PlainHistoryCell { lines } @@ -489,8 +549,6 @@ pub(crate) fn new_completed_mcp_tool_call( )); } } - - lines.push(Line::from("")); } Err(e) => { lines.push(Line::from(vec![ @@ -503,6 +561,8 @@ pub(crate) fn new_completed_mcp_tool_call( } }; + // Leading blank separator at the start of this cell + lines.insert(0, Line::from("")); Box::new(PlainHistoryCell { lines }) } @@ -512,6 +572,7 @@ pub(crate) fn new_status_output( session_id: &Option, ) -> PlainHistoryCell { let mut lines: Vec> = Vec::new(); + lines.push(Line::from("")); lines.push(Line::from("/status".magenta())); let config_entries = create_config_summary_entries(config); @@ -596,8 +657,6 @@ pub(crate) fn new_status_output( ])); } - lines.push(Line::from("")); - // 👤 Account (only if ChatGPT tokens exist), shown under the first block let auth_file = get_auth_file(&config.codex_home); if let Ok(auth) = try_read_auth_json(&auth_file) @@ -688,13 +747,13 @@ pub(crate) fn new_status_output( usage.blended_total().to_string().into(), ])); - lines.push(Line::from("")); PlainHistoryCell { lines } } /// Render a summary of configured MCP servers from the current `Config`. pub(crate) fn empty_mcp_output() -> PlainHistoryCell { let lines: Vec> = vec![ + Line::from(""), Line::from("/mcp".magenta()), Line::from(""), Line::from(vec!["🔌 ".into(), "MCP Tools".bold()]), @@ -709,7 +768,6 @@ pub(crate) fn empty_mcp_output() -> PlainHistoryCell { " to configure them.".into(), ]) .style(Style::default().add_modifier(Modifier::DIM)), - Line::from(""), ]; PlainHistoryCell { lines } @@ -782,7 +840,7 @@ pub(crate) fn new_mcp_tools_output( } pub(crate) fn new_error_event(message: String) -> PlainHistoryCell { - let lines: Vec> = vec![vec!["🖐 ".red().bold(), message.into()].into(), "".into()]; + let lines: Vec> = vec!["".into(), vec!["🖐 ".red().bold(), message.into()].into()]; PlainHistoryCell { lines } } @@ -797,6 +855,8 @@ pub(crate) fn new_plan_update(update: UpdatePlanArgs) -> PlainHistoryCell { let UpdatePlanArgs { explanation, plan } = update; let mut lines: Vec> = Vec::new(); + // Leading blank for separation + lines.push(Line::from("")); // Header with progress summary let total = plan.len(); let completed = plan @@ -887,8 +947,6 @@ pub(crate) fn new_plan_update(update: UpdatePlanArgs) -> PlainHistoryCell { } } - lines.push(Line::from("")); - PlainHistoryCell { lines } } @@ -908,16 +966,16 @@ pub(crate) fn new_patch_event( auto_approved: false, } => { let lines: Vec> = vec![ - Line::from("✏️ Applying patch".magenta().bold()), Line::from(""), + Line::from("✏️ Applying patch".magenta().bold()), ]; return PlainHistoryCell { lines }; } }; let mut lines: Vec> = create_diff_summary(title, &changes, event_type); - - lines.push(Line::from("")); + // Add leading blank separator for the cell + lines.insert(0, Line::from("")); PlainHistoryCell { lines } } @@ -934,14 +992,15 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell { exit_code: 1, stdout: String::new(), stderr, + formatted_output: String::new(), }), true, true, )); } - lines.push(Line::from("")); - + // Leading blank separator + lines.insert(0, Line::from("")); PlainHistoryCell { lines } } @@ -988,9 +1047,8 @@ pub(crate) fn new_patch_apply_success(stdout: String) -> PlainHistoryCell { lines.push(Line::from(format!("... +{remaining} lines")).dim()); } } - - lines.push(Line::from("")); - + // Leading blank separator + lines.insert(0, Line::from("")); PlainHistoryCell { lines } } @@ -999,9 +1057,9 @@ pub(crate) fn new_reasoning_block( config: &Config, ) -> TranscriptOnlyHistoryCell { let mut lines: Vec> = Vec::new(); + lines.push(Line::from("")); lines.push(Line::from("thinking".magenta().italic())); append_markdown(&full_reasoning_buffer, &mut lines, config); - lines.push(Line::from("")); TranscriptOnlyHistoryCell { lines } } @@ -1014,6 +1072,7 @@ fn output_lines( exit_code, stdout, stderr, + .. } = match output { Some(output) if only_err && output.exit_code == 0 => return vec![], Some(output) => output, @@ -1096,9 +1155,14 @@ mod tests { let parsed = vec![ParsedCommand::Unknown { cmd: "printf 'foo\nbar'".to_string(), }]; - let lines = exec_command_lines(&[], &parsed, None, None); - assert!(lines.len() >= 3); - assert_eq!(lines[1].spans[0].content, " └ "); - assert_eq!(lines[2].spans[0].content, " "); + let lines = exec_command_lines(&[], &parsed, None, None, true); + assert!(lines.len() >= 4); + // Leading spacer then header line + assert!(lines[0].spans.is_empty() || lines[0].spans[0].content.is_empty()); + assert_eq!(lines[1].spans[0].content, ">_"); + // First rendered command line starts with two-space + marker. + assert_eq!(lines[2].spans[0].content, " "); + // Continuation lines align under the text block. + assert_eq!(lines[3].spans[0].content, " "); } } diff --git a/codex-rs/tui/src/render/line_utils.rs b/codex-rs/tui/src/render/line_utils.rs index 7d11c54155..1cc9d5c95a 100644 --- a/codex-rs/tui/src/render/line_utils.rs +++ b/codex-rs/tui/src/render/line_utils.rs @@ -34,12 +34,3 @@ pub fn is_blank_line_spaces_only(line: &Line<'_>) -> bool { .iter() .all(|s| s.content.is_empty() || s.content.chars().all(|c| c == ' ')) } - -/// Consider a line blank if its spans are empty or all span contents are -/// whitespace when trimmed. -pub fn is_blank_line_trim(line: &Line<'_>) -> bool { - if line.spans.is_empty() { - return true; - } - line.spans.iter().all(|s| s.content.trim().is_empty()) -} diff --git a/codex-rs/tui/src/streaming/controller.rs b/codex-rs/tui/src/streaming/controller.rs index 5fb60e78a5..b9afa89392 100644 --- a/codex-rs/tui/src/streaming/controller.rs +++ b/codex-rs/tui/src/streaming/controller.rs @@ -70,17 +70,6 @@ impl StreamController { self.header.maybe_emit(out_lines) } - #[inline] - fn ensure_single_trailing_blank(lines: &mut Lines) { - if lines - .last() - .map(|l| !crate::render::line_utils::is_blank_line_trim(l)) - .unwrap_or(true) - { - lines.push(Line::from("")); - } - } - /// Begin an answer stream. Does not emit header yet; it is emitted on first commit. pub(crate) fn begin(&mut self, _sink: &impl HistorySink) { // Starting a new stream cancels any pending finish-from-previous-stream animation. @@ -138,7 +127,6 @@ impl StreamController { let mut lines_with_header: Lines = Vec::new(); self.emit_header_if_needed(&mut lines_with_header); lines_with_header.extend(out_lines); - Self::ensure_single_trailing_blank(&mut lines_with_header); sink.insert_history(lines_with_header); } diff --git a/codex-rs/tui/src/streaming/mod.rs b/codex-rs/tui/src/streaming/mod.rs index 1e670738f1..eaaeeff4be 100644 --- a/codex-rs/tui/src/streaming/mod.rs +++ b/codex-rs/tui/src/streaming/mod.rs @@ -64,6 +64,8 @@ impl HeaderEmitter { pub(crate) fn maybe_emit(&mut self, out_lines: &mut Vec>) -> bool { if !self.emitted_in_stream && !self.emitted_this_turn { + // Add a leading blank line before the header for visual spacing + out_lines.push(ratatui::text::Line::from("")); out_lines.push(render_header_line()); self.emitted_in_stream = true; self.emitted_this_turn = true; diff --git a/codex-rs/tui/src/user_approval_widget.rs b/codex-rs/tui/src/user_approval_widget.rs index deb2c8ff16..de22f12b7b 100644 --- a/codex-rs/tui/src/user_approval_widget.rs +++ b/codex-rs/tui/src/user_approval_widget.rs @@ -258,7 +258,7 @@ impl UserApprovalWidget { } fn send_decision_with_feedback(&mut self, decision: ReviewDecision, feedback: String) { - let mut lines: Vec> = Vec::new(); + let mut lines: Vec> = vec![Line::from("")]; match &self.approval_request { ApprovalRequest::Exec { command, .. } => { let cmd = strip_bash_lc_and_escape(command); @@ -327,7 +327,6 @@ impl UserApprovalWidget { lines.push(Line::from(l.to_string())); } } - lines.push(Line::from("")); self.app_event_tx.send(AppEvent::InsertHistoryLines(lines)); let op = match &self.approval_request { diff --git a/codex-rs/tui/tests/fixtures/ideal-binary-response.txt b/codex-rs/tui/tests/fixtures/ideal-binary-response.txt index 80aae5d0e4..62d6af570f 100644 --- a/codex-rs/tui/tests/fixtures/ideal-binary-response.txt +++ b/codex-rs/tui/tests/fixtures/ideal-binary-response.txt @@ -9,7 +9,8 @@ codex I’m going to scan the workspace and Cargo manifests to see build profiles and dependencies that impact binary size. Then I’ll summarize the main causes. -⚡ Running ls -la +>_ + ✓ ls -la └ total 6696 drwxr-xr-x@ 39 easong staff 1248 Aug 9 08:49 . drwxr-xr-x@ 29 easong staff 928 Aug 9 08:50 .. @@ -21,7 +22,7 @@ dependencies that impact binary size. Then I’ll summarize the main causes. drwxr-xr-x@ 6 easong staff 192 Aug 7 19:20 tui -rw-r--r--@ 1 easong staff 8639 Aug 6 23:30 typewriter.md -rw-r--r--@ 1 easong staff 7673 Aug 6 21:58 wrap-plan.md -⚡ Running sed -n '1,200p' Cargo.toml + ✓ sed -n '1,200p' Cargo.toml └ [workspace] members = [ "ansi-escape", @@ -34,7 +35,7 @@ dependencies that impact binary size. Then I’ll summarize the main causes. # ratatui = { path = "../../ratatui" } ratatui = { git = "https://github.com/nornagon/ratatui", branch = "nornagon-v0.29.0-patch" } -⚡ Running for d in ansi-escape apply-patch arg0 cli common core exec execpolicy + ✓ for d in ansi-escape apply-patch arg0 cli common core exec execpolicy file-search linux-sandbox login mcp-client mcp-server mcp-types ollama tui; do echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done └ --- ansi-escape/Cargo.toml @@ -48,7 +49,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done "signal", ] } tracing = { version -⚡ Running sed -n '1,200p' execpolicy/Cargo.toml + ✓ sed -n '1,200p' execpolicy/Cargo.toml └ [package] name = "codex-execpolicy" version = { workspace = true } @@ -60,7 +61,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done [dev-dependencies] tempfile = "3.13.0" -⚡ Running sed -n '1,200p' file-search/Cargo.toml + ✓ sed -n '1,200p' file-search/Cargo.toml └ [package] edition = "2024" name = "codex-file-search" @@ -72,7 +73,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done serde = { version = "1", features = ["derive"] } serde_json = "1.0.142" tokio = { version = "1", features = ["full"] } -⚡ Running sed -n '1,200p' linux-sandbox/Cargo.toml + ✓ sed -n '1,200p' linux-sandbox/Cargo.toml └ [package] edition = "2024" name = "codex-linux-sandbox" @@ -84,7 +85,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done "rt-multi-thread", "signal", ] } -⚡ Running sed -n '1,200p' login/Cargo.toml + ✓ sed -n '1,200p' login/Cargo.toml └ [package] edition = "2024" name = "codex-login" @@ -96,7 +97,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done [dev-dependencies] pretty_assertions = "1.4.1" tempfile = "3" -⚡ Running sed -n '1,200p' mcp-client/Cargo.toml + ✓ sed -n '1,200p' mcp-client/Cargo.toml └ [package] name = "codex-mcp-client" version = { workspace = true } @@ -108,7 +109,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done "sync", "time", ] } -⚡ Running sed -n '1,200p' mcp-server/Cargo.toml + ✓ sed -n '1,200p' mcp-server/Cargo.toml └ [package] edition = "2024" name = "codex-mcp-server" @@ -120,7 +121,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done tempfile = "3" tokio-test = "0.4" wiremock = "0.6" -⚡ Running sed -n '1,200p' mcp-types/Cargo.toml + ✓ sed -n '1,200p' mcp-types/Cargo.toml └ [package] edition = "2024" name = "mcp-types" @@ -132,7 +133,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done [dependencies] serde = { version = "1", features = ["derive"] } serde_json = "1" -⚡ Running sed -n '1,200p' ollama/Cargo.toml + ✓ sed -n '1,200p' ollama/Cargo.toml └ [package] edition = "2024" name = "codex-ollama" @@ -144,7 +145,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done [dev-dependencies] tempfile = "3" -⚡ Running sed -n '1,200p' tui/Cargo.toml + ✓ sed -n '1,200p' tui/Cargo.toml └ [package] edition = "2024" name = "codex-tui" @@ -156,7 +157,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done pretty_assertions = "1" rand = "0.8" vt100 = "0.16.2" -⚡ Running sed -n '1,200p' chatgpt/Cargo.toml + ✓ sed -n '1,200p' chatgpt/Cargo.toml └ [package] edition = "2024" name = "codex-chatgpt" @@ -168,6 +169,7 @@ echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done [dev-dependencies] tempfile = "3" + codex Here’s what’s driving size in this workspace’s binaries. From 5fa7d46ddf4a035288cc5afadfe883a597ac4287 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Fri, 22 Aug 2025 16:33:18 -0700 Subject: [PATCH 11/16] tui: fix resize on wezterm (#2600) WezTerm doesn't respond to cursor queries during a synchronized update, so resizing was broken there. --- codex-rs/tui/src/tui.rs | 68 +++++++++++++++++++++++++++++------------ 1 file changed, 49 insertions(+), 19 deletions(-) diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 4498f46e17..d30e9a6e17 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -138,6 +138,12 @@ enum ResumeAction { RestoreAlt = 2, } +#[cfg(unix)] +enum PreparedResumeAction { + RestoreAltScreen, + RealignViewport(ratatui::layout::Rect), +} + #[cfg(unix)] fn take_resume_action(pending: &AtomicU8) -> ResumeAction { match pending.swap(ResumeAction::None as u8, Ordering::Relaxed) { @@ -333,23 +339,37 @@ impl Tui { } #[cfg(unix)] - fn apply_resume_action(&mut self, action: ResumeAction) -> Result<()> { + fn prepare_resume_action( + &mut self, + action: ResumeAction, + ) -> Result> { match action { ResumeAction::RealignInline => { let cursor_pos = self.terminal.get_cursor_position()?; - self.terminal - .set_viewport_area(ratatui::layout::Rect::new(0, cursor_pos.y, 0, 0)); + Ok(Some(PreparedResumeAction::RealignViewport( + ratatui::layout::Rect::new(0, cursor_pos.y, 0, 0), + ))) } ResumeAction::RestoreAlt => { - // When we're resuming from alt screen, we need to save what the cursor position - // _was_ when we resumed. That way, when we leave the alt screen, we can restore - // the cursor to the new position. if let Ok((_x, y)) = crossterm::cursor::position() && let Some(saved) = self.alt_saved_viewport.as_mut() { saved.y = y; } - let _ = execute!(self.terminal.backend_mut(), EnterAlternateScreen); + Ok(Some(PreparedResumeAction::RestoreAltScreen)) + } + ResumeAction::None => Ok(None), + } + } + + #[cfg(unix)] + fn apply_prepared_resume_action(&mut self, prepared: PreparedResumeAction) -> Result<()> { + match prepared { + PreparedResumeAction::RealignViewport(area) => { + self.terminal.set_viewport_area(area); + } + PreparedResumeAction::RestoreAltScreen => { + execute!(self.terminal.backend_mut(), EnterAlternateScreen)?; if let Ok(size) = self.terminal.size() { self.terminal.set_viewport_area(ratatui::layout::Rect::new( 0, @@ -360,13 +380,10 @@ impl Tui { self.terminal.clear()?; } } - ResumeAction::None => {} } Ok(()) } - // Public suspend() removed; Ctrl+Z is handled internally via event_stream + draw. - /// Enter alternate screen and expand the viewport to full terminal size, saving the current /// inline viewport for restoration when leaving. pub fn enter_alt_screen(&mut self) -> Result<()> { @@ -405,12 +422,13 @@ impl Tui { height: u16, draw_fn: impl FnOnce(&mut custom_terminal::Frame), ) -> Result<()> { - std::io::stdout().sync_update(|_| { - #[cfg(unix)] - { - // Apply any post-resume action before layout/clear/draw. - self.apply_resume_action(take_resume_action(&self.resume_pending))?; - } + // Precompute any viewport updates that need a cursor-position query before entering + // the synchronized update, to avoid racing with the event reader. + let mut pending_viewport_area: Option = None; + #[cfg(unix)] + let mut prepared_resume = + self.prepare_resume_action(take_resume_action(&self.resume_pending))?; + { let terminal = &mut self.terminal; let screen_size = terminal.size()?; let last_known_screen_size = terminal.last_known_screen_size; @@ -419,15 +437,27 @@ impl Tui { let last_known_cursor_pos = terminal.last_known_cursor_pos; if cursor_pos.y != last_known_cursor_pos.y { let cursor_delta = cursor_pos.y as i32 - last_known_cursor_pos.y as i32; - let new_viewport_area = terminal.viewport_area.offset(Offset { x: 0, y: cursor_delta, }); - terminal.set_viewport_area(new_viewport_area); - terminal.clear()?; + pending_viewport_area = Some(new_viewport_area); } } + } + + std::io::stdout().sync_update(|_| { + #[cfg(unix)] + { + if let Some(prepared) = prepared_resume.take() { + self.apply_prepared_resume_action(prepared)?; + } + } + let terminal = &mut self.terminal; + if let Some(new_area) = pending_viewport_area.take() { + terminal.set_viewport_area(new_area); + terminal.clear()?; + } let size = terminal.size()?; From 311ad0ce267e109853dfb30a4ce93e3f7f011f17 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Fri, 22 Aug 2025 17:06:09 -0700 Subject: [PATCH 12/16] fork conversation from a previous message (#2575) This can be the underlying logic in order to start a conversation from a previous message. will need some love in the UI. Base for building this: #2588 --- codex-rs/core/src/codex.rs | 31 ++++- codex-rs/core/src/conversation_manager.rs | 129 +++++++++++++++++- .../src/event_processor_with_human_output.rs | 1 + codex-rs/mcp-server/src/codex_tool_runner.rs | 1 + codex-rs/protocol/src/protocol.rs | 15 ++ codex-rs/tui/src/chatwidget.rs | 1 + 6 files changed, 174 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index cdf94210bd..b08c8e4a03 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -14,6 +14,7 @@ use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::MaybeApplyPatchVerified; use codex_apply_patch::maybe_parse_apply_patch_verified; use codex_login::AuthManager; +use codex_protocol::protocol::ConversationHistoryResponseEvent; use codex_protocol::protocol::TurnAbortReason; use codex_protocol::protocol::TurnAbortedEvent; use futures::prelude::*; @@ -147,6 +148,7 @@ impl Codex { pub async fn spawn( config: Config, auth_manager: Arc, + initial_history: Option>, ) -> CodexResult { let (tx_sub, rx_sub) = async_channel::bounded(64); let (tx_event, rx_event) = async_channel::unbounded(); @@ -177,6 +179,7 @@ impl Codex { config.clone(), auth_manager.clone(), tx_event.clone(), + initial_history, ) .await .map_err(|e| { @@ -186,7 +189,12 @@ impl Codex { let session_id = session.session_id; // This task will run until Op::Shutdown is received. - tokio::spawn(submission_loop(session, turn_context, config, rx_sub)); + tokio::spawn(submission_loop( + session.clone(), + turn_context, + config, + rx_sub, + )); let codex = Codex { next_id: AtomicU64::new(0), tx_sub, @@ -332,6 +340,7 @@ impl Session { config: Arc, auth_manager: Arc, tx_event: Sender, + initial_history: Option>, ) -> anyhow::Result<(Arc, TurnContext)> { let ConfigureSession { provider, @@ -391,14 +400,15 @@ impl Session { } let rollout_result = match rollout_res { Ok((session_id, maybe_saved, recorder)) => { - let restored_items: Option> = + let restored_items: Option> = initial_history.or_else(|| { maybe_saved.and_then(|saved_session| { if saved_session.items.is_empty() { None } else { Some(saved_session.items) } - }); + }) + }); RolloutResult { session_id, rollout_recorder: Some(recorder), @@ -1286,6 +1296,21 @@ async fn submission_loop( } break; } + Op::GetHistory => { + let tx_event = sess.tx_event.clone(); + let sub_id = sub.id.clone(); + + let event = Event { + id: sub_id.clone(), + msg: EventMsg::ConversationHistory(ConversationHistoryResponseEvent { + conversation_id: sess.session_id, + entries: sess.state.lock_unchecked().history.contents(), + }), + }; + if let Err(e) = tx_event.send(event).await { + warn!("failed to send ConversationHistory event: {e}"); + } + } _ => { // Ignore unknown ops; enum is non_exhaustive to allow extensions. } diff --git a/codex-rs/core/src/conversation_manager.rs b/codex-rs/core/src/conversation_manager.rs index b553843125..fd90f54660 100644 --- a/codex-rs/core/src/conversation_manager.rs +++ b/codex-rs/core/src/conversation_manager.rs @@ -16,6 +16,7 @@ use crate::error::Result as CodexResult; use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::SessionConfiguredEvent; +use codex_protocol::models::ResponseItem; /// Represents a newly created Codex conversation, including the first event /// (which is [`EventMsg::SessionConfigured`]). @@ -59,8 +60,18 @@ impl ConversationManager { let CodexSpawnOk { codex, session_id: conversation_id, - } = Codex::spawn(config, auth_manager).await?; + } = { + let initial_history = None; + Codex::spawn(config, auth_manager, initial_history).await? + }; + self.finalize_spawn(codex, conversation_id).await + } + async fn finalize_spawn( + &self, + codex: Codex, + conversation_id: Uuid, + ) -> CodexResult { // The first event must be `SessionInitialized`. Validate and forward it // to the caller so that they can display it in the conversation // history. @@ -98,4 +109,120 @@ impl ConversationManager { .cloned() .ok_or_else(|| CodexErr::ConversationNotFound(conversation_id)) } + + /// Fork an existing conversation by dropping the last `drop_last_messages` + /// user/assistant messages from its transcript and starting a new + /// conversation with identical configuration (unless overridden by the + /// caller's `config`). The new conversation will have a fresh id. + pub async fn fork_conversation( + &self, + conversation_history: Vec, + num_messages_to_drop: usize, + config: Config, + ) -> CodexResult { + // Compute the prefix up to the cut point. + let truncated_history = + truncate_after_dropping_last_messages(conversation_history, num_messages_to_drop); + + // Spawn a new conversation with the computed initial history. + let auth_manager = self.auth_manager.clone(); + let CodexSpawnOk { + codex, + session_id: conversation_id, + } = Codex::spawn(config, auth_manager, Some(truncated_history)).await?; + + self.finalize_spawn(codex, conversation_id).await + } +} + +/// Return a prefix of `items` obtained by dropping the last `n` user messages +/// and all items that follow them. +fn truncate_after_dropping_last_messages(items: Vec, n: usize) -> Vec { + if n == 0 || items.is_empty() { + return items; + } + + // Walk backwards counting only `user` Message items, find cut index. + let mut count = 0usize; + let mut cut_index = 0usize; + for (idx, item) in items.iter().enumerate().rev() { + if let ResponseItem::Message { role, .. } = item + && role == "user" + { + count += 1; + if count == n { + // Cut everything from this user message to the end. + cut_index = idx; + break; + } + } + } + if count < n { + // If fewer than n messages exist, drop everything. + Vec::new() + } else { + items.into_iter().take(cut_index).collect() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use codex_protocol::models::ContentItem; + use codex_protocol::models::ReasoningItemReasoningSummary; + use codex_protocol::models::ResponseItem; + + fn user_msg(text: &str) -> ResponseItem { + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::OutputText { + text: text.to_string(), + }], + } + } + fn assistant_msg(text: &str) -> ResponseItem { + ResponseItem::Message { + id: None, + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { + text: text.to_string(), + }], + } + } + + #[test] + fn drops_from_last_user_only() { + let items = vec![ + user_msg("u1"), + assistant_msg("a1"), + assistant_msg("a2"), + user_msg("u2"), + assistant_msg("a3"), + ResponseItem::Reasoning { + id: "r1".to_string(), + summary: vec![ReasoningItemReasoningSummary::SummaryText { + text: "s".to_string(), + }], + content: None, + encrypted_content: None, + }, + ResponseItem::FunctionCall { + id: None, + name: "tool".to_string(), + arguments: "{}".to_string(), + call_id: "c1".to_string(), + }, + assistant_msg("a4"), + ]; + + let truncated = truncate_after_dropping_last_messages(items.clone(), 1); + assert_eq!( + truncated, + vec![items[0].clone(), items[1].clone(), items[2].clone()] + ); + + let truncated2 = truncate_after_dropping_last_messages(items, 2); + assert!(truncated2.is_empty()); + } } diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 1a42fd0c61..0f3b56b402 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -540,6 +540,7 @@ impl EventProcessor for EventProcessorWithHumanOutput { } }, EventMsg::ShutdownComplete => return CodexStatus::Shutdown, + EventMsg::ConversationHistory(_) => {} } CodexStatus::Running } diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 36845d895c..c6d65bc89d 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -275,6 +275,7 @@ async fn run_codex_tool_session_inner( | EventMsg::GetHistoryEntryResponse(_) | EventMsg::PlanUpdate(_) | EventMsg::TurnAborted(_) + | EventMsg::ConversationHistory(_) | EventMsg::ShutdownComplete => { // For now, we do not do anything extra for these // events. Note that diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index f337f94fe6..e803324a2d 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -22,6 +22,7 @@ use uuid::Uuid; use crate::config_types::ReasoningEffort as ReasoningEffortConfig; use crate::config_types::ReasoningSummary as ReasoningSummaryConfig; use crate::message_history::HistoryEntry; +use crate::models::ResponseItem; use crate::parse_command::ParsedCommand; use crate::plan_tool::UpdatePlanArgs; @@ -137,6 +138,10 @@ pub enum Op { /// Request a single history entry identified by `log_id` + `offset`. GetHistoryEntryRequest { offset: usize, log_id: u64 }, + /// Request the full in-memory conversation transcript for the current session. + /// Reply is delivered via `EventMsg::ConversationHistory`. + GetHistory, + /// Request the list of MCP tools available across all configured servers. /// Reply is delivered via `EventMsg::McpListToolsResponse`. ListMcpTools, @@ -471,6 +476,8 @@ pub enum EventMsg { /// Notification that the agent is shutting down. ShutdownComplete, + + ConversationHistory(ConversationHistoryResponseEvent), } // Individual event payload types matching each `EventMsg` variant. @@ -651,6 +658,14 @@ impl McpToolCallEndEvent { } } +/// Response payload for `Op::GetHistory` containing the current session's +/// in-memory transcript. +#[derive(Debug, Clone, Deserialize, Serialize)] +pub struct ConversationHistoryResponseEvent { + pub conversation_id: Uuid, + pub entries: Vec, +} + #[derive(Debug, Clone, Deserialize, Serialize)] pub struct ExecCommandBeginEvent { /// Identifier so this can be paired with the ExecCommandEnd event. diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index e3097c9162..3f24a0176c 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -839,6 +839,7 @@ impl ChatWidget { self.on_background_event(message) } EventMsg::StreamError(StreamErrorEvent { message }) => self.on_stream_error(message), + EventMsg::ConversationHistory(_) => {} } // Coalesce redraws: issue at most one after handling the event if self.needs_redraw { From e3b03eaccb104e50b08a3008eb209486babe05ed Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 22 Aug 2025 18:10:55 -0700 Subject: [PATCH 13/16] feat: StreamableShell with exec_command and write_stdin tools (#2574) --- codex-rs/Cargo.lock | 75 ++ codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/codex.rs | 54 ++ codex-rs/core/src/config.rs | 10 + .../src/exec_command/exec_command_params.rs | 57 ++ .../src/exec_command/exec_command_session.rs | 83 +++ codex-rs/core/src/exec_command/mod.rs | 14 + .../core/src/exec_command/responses_api.rs | 98 +++ codex-rs/core/src/exec_command/session_id.rs | 5 + .../core/src/exec_command/session_manager.rs | 677 ++++++++++++++++++ codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/openai_tools.rs | 23 +- 12 files changed, 1096 insertions(+), 2 deletions(-) create mode 100644 codex-rs/core/src/exec_command/exec_command_params.rs create mode 100644 codex-rs/core/src/exec_command/exec_command_session.rs create mode 100644 codex-rs/core/src/exec_command/mod.rs create mode 100644 codex-rs/core/src/exec_command/responses_api.rs create mode 100644 codex-rs/core/src/exec_command/session_id.rs create mode 100644 codex-rs/core/src/exec_command/session_manager.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 94702a83ed..dbccbd863e 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -731,6 +731,7 @@ dependencies = [ "mime_guess", "openssl-sys", "os_info", + "portable-pty", "predicates", "pretty_assertions", "rand 0.9.2", @@ -1479,6 +1480,12 @@ version = "0.15.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1aaf95b3e5c8f23aa320147307562d361db0ae0d51242340f558153b4eb2439b" +[[package]] +name = "downcast-rs" +version = "1.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75b325c5dbd37f80359721ad39aca5a29fb04c89279657cffdda8736d0c0b9d2" + [[package]] name = "dupe" version = "0.9.1" @@ -1724,6 +1731,17 @@ dependencies = [ "simd-adler32", ] +[[package]] +name = "filedescriptor" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e40758ed24c9b2eeb76c35fb0aebc66c626084edd827e07e1552279814c6682d" +dependencies = [ + "libc", + "thiserror 1.0.69", + "winapi", +] + [[package]] name = "fixedbitset" version = "0.4.2" @@ -3439,6 +3457,27 @@ dependencies = [ "portable-atomic", ] +[[package]] +name = "portable-pty" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4a596a2b3d2752d94f51fac2d4a96737b8705dddd311a32b9af47211f08671e" +dependencies = [ + "anyhow", + "bitflags 1.3.2", + "downcast-rs", + "filedescriptor", + "lazy_static", + "libc", + "log", + "nix", + "serial2", + "shared_library", + "shell-words", + "winapi", + "winreg", +] + [[package]] name = "potential_utf" version = "0.1.2" @@ -4366,6 +4405,17 @@ dependencies = [ "syn 2.0.104", ] +[[package]] +name = "serial2" +version = "0.2.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26e1e5956803a69ddd72ce2de337b577898801528749565def03515f82bad5bb" +dependencies = [ + "cfg-if", + "libc", + "winapi", +] + [[package]] name = "sha1" version = "0.10.6" @@ -4397,6 +4447,22 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "shared_library" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a9e7e0f2bfae24d8a5b5a66c5b257a83c7412304311512a0c054cd5e619da11" +dependencies = [ + "lazy_static", + "libc", +] + +[[package]] +name = "shell-words" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24188a676b6ae68c3b2cb3a01be17fbf7240ce009799bb56d5b1409051e78fde" + [[package]] name = "shlex" version = "1.3.0" @@ -6176,6 +6242,15 @@ dependencies = [ "memchr", ] +[[package]] +name = "winreg" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "80d0f4e272c85def139476380b12f9ac60926689dd2e01d4923222f40580869d" +dependencies = [ + "winapi", +] + [[package]] name = "winsafe" version = "0.0.19" diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 56815ba03c..2f2fa7cbad 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -28,6 +28,7 @@ libc = "0.2.175" mcp-types = { path = "../mcp-types" } mime_guess = "2.0" os_info = "3.12.0" +portable-pty = "0.9.0" rand = "0.9" regex-lite = "0.1.6" reqwest = { version = "0.12", features = ["json", "stream"] } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index b08c8e4a03..ec053ce4aa 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -53,6 +53,11 @@ use crate::exec::SandboxType; use crate::exec::StdoutStream; use crate::exec::StreamOutput; use crate::exec::process_exec_tool_call; +use crate::exec_command::EXEC_COMMAND_TOOL_NAME; +use crate::exec_command::ExecCommandParams; +use crate::exec_command::SESSION_MANAGER; +use crate::exec_command::WRITE_STDIN_TOOL_NAME; +use crate::exec_command::WriteStdinParams; use crate::exec_env::create_env; use crate::mcp_connection_manager::McpConnectionManager; use crate::mcp_tool_call::handle_mcp_tool_call; @@ -498,6 +503,7 @@ impl Session { sandbox_policy.clone(), config.include_plan_tool, config.include_apply_patch_tool, + config.use_experimental_streamable_shell_tool, ), user_instructions, base_instructions, @@ -1080,6 +1086,7 @@ async fn submission_loop( new_sandbox_policy.clone(), config.include_plan_tool, config.include_apply_patch_tool, + config.use_experimental_streamable_shell_tool, ); let new_turn_context = TurnContext { @@ -1158,6 +1165,7 @@ async fn submission_loop( sandbox_policy.clone(), config.include_plan_tool, config.include_apply_patch_tool, + config.use_experimental_streamable_shell_tool, ), user_instructions: turn_context.user_instructions.clone(), base_instructions: turn_context.base_instructions.clone(), @@ -2063,6 +2071,52 @@ async fn handle_function_call( .await } "update_plan" => handle_update_plan(sess, arguments, sub_id, call_id).await, + EXEC_COMMAND_TOOL_NAME => { + // TODO(mbolin): Sandbox check. + let exec_params = match serde_json::from_str::(&arguments) { + Ok(params) => params, + Err(e) => { + return ResponseInputItem::FunctionCallOutput { + call_id, + output: FunctionCallOutputPayload { + content: format!("failed to parse function arguments: {e}"), + success: Some(false), + }, + }; + } + }; + let result = SESSION_MANAGER + .handle_exec_command_request(exec_params) + .await; + let function_call_output = crate::exec_command::result_into_payload(result); + ResponseInputItem::FunctionCallOutput { + call_id, + output: function_call_output, + } + } + WRITE_STDIN_TOOL_NAME => { + let write_stdin_params = match serde_json::from_str::(&arguments) { + Ok(params) => params, + Err(e) => { + return ResponseInputItem::FunctionCallOutput { + call_id, + output: FunctionCallOutputPayload { + content: format!("failed to parse function arguments: {e}"), + success: Some(false), + }, + }; + } + }; + let result = SESSION_MANAGER + .handle_write_stdin_request(write_stdin_params) + .await; + let function_call_output: FunctionCallOutputPayload = + crate::exec_command::result_into_payload(result); + ResponseInputItem::FunctionCallOutput { + call_id, + output: function_call_output, + } + } _ => { match sess.mcp_connection_manager.parse_tool_name(&name) { Some((server, tool_name)) => { diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 67a54eb1d7..fbf0387a01 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -174,6 +174,8 @@ pub struct Config { /// If set to `true`, the API key will be signed with the `originator` header. pub preferred_auth_method: AuthMode, + + pub use_experimental_streamable_shell_tool: bool, } impl Config { @@ -469,6 +471,8 @@ pub struct ConfigToml { /// Experimental path to a file whose contents replace the built-in BASE_INSTRUCTIONS. pub experimental_instructions_file: Option, + pub experimental_use_exec_command_tool: Option, + /// The value for the `originator` header included with Responses API requests. pub responses_originator_header_internal_override: Option, @@ -758,6 +762,9 @@ impl Config { include_apply_patch_tool: include_apply_patch_tool.unwrap_or(false), responses_originator_header, preferred_auth_method: cfg.preferred_auth_method.unwrap_or(AuthMode::ChatGPT), + use_experimental_streamable_shell_tool: cfg + .experimental_use_exec_command_tool + .unwrap_or(false), }; Ok(config) } @@ -1124,6 +1131,7 @@ disable_response_storage = true include_apply_patch_tool: false, responses_originator_header: "codex_cli_rs".to_string(), preferred_auth_method: AuthMode::ChatGPT, + use_experimental_streamable_shell_tool: false, }, o3_profile_config ); @@ -1178,6 +1186,7 @@ disable_response_storage = true include_apply_patch_tool: false, responses_originator_header: "codex_cli_rs".to_string(), preferred_auth_method: AuthMode::ChatGPT, + use_experimental_streamable_shell_tool: false, }; assert_eq!(expected_gpt3_profile_config, gpt3_profile_config); @@ -1247,6 +1256,7 @@ disable_response_storage = true include_apply_patch_tool: false, responses_originator_header: "codex_cli_rs".to_string(), preferred_auth_method: AuthMode::ChatGPT, + use_experimental_streamable_shell_tool: false, }; assert_eq!(expected_zdr_profile_config, zdr_profile_config); diff --git a/codex-rs/core/src/exec_command/exec_command_params.rs b/codex-rs/core/src/exec_command/exec_command_params.rs new file mode 100644 index 0000000000..11a3fd4596 --- /dev/null +++ b/codex-rs/core/src/exec_command/exec_command_params.rs @@ -0,0 +1,57 @@ +use serde::Deserialize; +use serde::Serialize; + +use crate::exec_command::session_id::SessionId; + +#[derive(Debug, Clone, Deserialize)] +pub struct ExecCommandParams { + pub(crate) cmd: String, + + #[serde(default = "default_yield_time")] + pub(crate) yield_time_ms: u64, + + #[serde(default = "max_output_tokens")] + pub(crate) max_output_tokens: u64, + + #[serde(default = "default_shell")] + pub(crate) shell: String, + + #[serde(default = "default_login")] + pub(crate) login: bool, +} + +fn default_yield_time() -> u64 { + 10_000 +} + +fn max_output_tokens() -> u64 { + 10_000 +} + +fn default_login() -> bool { + true +} + +fn default_shell() -> String { + "/bin/bash".to_string() +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct WriteStdinParams { + pub(crate) session_id: SessionId, + pub(crate) chars: String, + + #[serde(default = "write_stdin_default_yield_time_ms")] + pub(crate) yield_time_ms: u64, + + #[serde(default = "write_stdin_default_max_output_tokens")] + pub(crate) max_output_tokens: u64, +} + +fn write_stdin_default_yield_time_ms() -> u64 { + 250 +} + +fn write_stdin_default_max_output_tokens() -> u64 { + 10_000 +} diff --git a/codex-rs/core/src/exec_command/exec_command_session.rs b/codex-rs/core/src/exec_command/exec_command_session.rs new file mode 100644 index 0000000000..7503150c9a --- /dev/null +++ b/codex-rs/core/src/exec_command/exec_command_session.rs @@ -0,0 +1,83 @@ +use std::sync::Mutex as StdMutex; + +use tokio::sync::broadcast; +use tokio::sync::mpsc; +use tokio::task::JoinHandle; + +#[derive(Debug)] +pub(crate) struct ExecCommandSession { + /// Queue for writing bytes to the process stdin (PTY master write side). + writer_tx: mpsc::Sender>, + /// Broadcast stream of output chunks read from the PTY. New subscribers + /// receive only chunks emitted after they subscribe. + output_tx: broadcast::Sender>, + + /// Child killer handle for termination on drop (can signal independently + /// of a thread blocked in `.wait()`). + killer: StdMutex>>, + + /// JoinHandle for the blocking PTY reader task. + reader_handle: StdMutex>>, + + /// JoinHandle for the stdin writer task. + writer_handle: StdMutex>>, + + /// JoinHandle for the child wait task. + wait_handle: StdMutex>>, +} + +impl ExecCommandSession { + pub(crate) fn new( + writer_tx: mpsc::Sender>, + output_tx: broadcast::Sender>, + killer: Box, + reader_handle: JoinHandle<()>, + writer_handle: JoinHandle<()>, + wait_handle: JoinHandle<()>, + ) -> Self { + Self { + writer_tx, + output_tx, + killer: StdMutex::new(Some(killer)), + reader_handle: StdMutex::new(Some(reader_handle)), + writer_handle: StdMutex::new(Some(writer_handle)), + wait_handle: StdMutex::new(Some(wait_handle)), + } + } + + pub(crate) fn writer_sender(&self) -> mpsc::Sender> { + self.writer_tx.clone() + } + + pub(crate) fn output_receiver(&self) -> broadcast::Receiver> { + self.output_tx.subscribe() + } +} + +impl Drop for ExecCommandSession { + fn drop(&mut self) { + // Best-effort: terminate child first so blocking tasks can complete. + if let Ok(mut killer_opt) = self.killer.lock() + && let Some(mut killer) = killer_opt.take() + { + let _ = killer.kill(); + } + + // Abort background tasks; they may already have exited after kill. + if let Ok(mut h) = self.reader_handle.lock() + && let Some(handle) = h.take() + { + handle.abort(); + } + if let Ok(mut h) = self.writer_handle.lock() + && let Some(handle) = h.take() + { + handle.abort(); + } + if let Ok(mut h) = self.wait_handle.lock() + && let Some(handle) = h.take() + { + handle.abort(); + } + } +} diff --git a/codex-rs/core/src/exec_command/mod.rs b/codex-rs/core/src/exec_command/mod.rs new file mode 100644 index 0000000000..2fd88d4ec5 --- /dev/null +++ b/codex-rs/core/src/exec_command/mod.rs @@ -0,0 +1,14 @@ +mod exec_command_params; +mod exec_command_session; +mod responses_api; +mod session_id; +mod session_manager; + +pub use exec_command_params::ExecCommandParams; +pub use exec_command_params::WriteStdinParams; +pub use responses_api::EXEC_COMMAND_TOOL_NAME; +pub use responses_api::WRITE_STDIN_TOOL_NAME; +pub use responses_api::create_exec_command_tool_for_responses_api; +pub use responses_api::create_write_stdin_tool_for_responses_api; +pub use session_manager::SESSION_MANAGER; +pub use session_manager::result_into_payload; diff --git a/codex-rs/core/src/exec_command/responses_api.rs b/codex-rs/core/src/exec_command/responses_api.rs new file mode 100644 index 0000000000..70b90dd425 --- /dev/null +++ b/codex-rs/core/src/exec_command/responses_api.rs @@ -0,0 +1,98 @@ +use std::collections::BTreeMap; + +use crate::openai_tools::JsonSchema; +use crate::openai_tools::ResponsesApiTool; + +pub const EXEC_COMMAND_TOOL_NAME: &str = "exec_command"; +pub const WRITE_STDIN_TOOL_NAME: &str = "write_stdin"; + +pub fn create_exec_command_tool_for_responses_api() -> ResponsesApiTool { + let mut properties = BTreeMap::::new(); + properties.insert( + "cmd".to_string(), + JsonSchema::String { + description: Some("The shell command to execute.".to_string()), + }, + ); + properties.insert( + "yield_time_ms".to_string(), + JsonSchema::Number { + description: Some("The maximum time in milliseconds to wait for output.".to_string()), + }, + ); + properties.insert( + "max_output_tokens".to_string(), + JsonSchema::Number { + description: Some("The maximum number of tokens to output.".to_string()), + }, + ); + properties.insert( + "shell".to_string(), + JsonSchema::String { + description: Some("The shell to use. Defaults to \"/bin/bash\".".to_string()), + }, + ); + properties.insert( + "login".to_string(), + JsonSchema::Boolean { + description: Some( + "Whether to run the command as a login shell. Defaults to true.".to_string(), + ), + }, + ); + + ResponsesApiTool { + name: EXEC_COMMAND_TOOL_NAME.to_owned(), + description: r#"Execute shell commands on the local machine with streaming output."# + .to_string(), + strict: false, + parameters: JsonSchema::Object { + properties, + required: Some(vec!["cmd".to_string()]), + additional_properties: Some(false), + }, + } +} + +pub fn create_write_stdin_tool_for_responses_api() -> ResponsesApiTool { + let mut properties = BTreeMap::::new(); + properties.insert( + "session_id".to_string(), + JsonSchema::Number { + description: Some("The ID of the exec_command session.".to_string()), + }, + ); + properties.insert( + "chars".to_string(), + JsonSchema::String { + description: Some("The characters to write to stdin.".to_string()), + }, + ); + properties.insert( + "yield_time_ms".to_string(), + JsonSchema::Number { + description: Some( + "The maximum time in milliseconds to wait for output after writing.".to_string(), + ), + }, + ); + properties.insert( + "max_output_tokens".to_string(), + JsonSchema::Number { + description: Some("The maximum number of tokens to output.".to_string()), + }, + ); + + ResponsesApiTool { + name: WRITE_STDIN_TOOL_NAME.to_owned(), + description: r#"Write characters to an exec session's stdin. Returns all stdout+stderr received within yield_time_ms. +Can write control characters (\u0003 for Ctrl-C), or an empty string to just poll stdout+stderr."# + .to_string(), + strict: false, + parameters: JsonSchema::Object { + properties, + required: Some(vec!["session_id".to_string(), "chars".to_string()]), + additional_properties: Some(false), + }, + } +} diff --git a/codex-rs/core/src/exec_command/session_id.rs b/codex-rs/core/src/exec_command/session_id.rs new file mode 100644 index 0000000000..c97c5d5440 --- /dev/null +++ b/codex-rs/core/src/exec_command/session_id.rs @@ -0,0 +1,5 @@ +use serde::Deserialize; +use serde::Serialize; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub(crate) struct SessionId(pub u32); diff --git a/codex-rs/core/src/exec_command/session_manager.rs b/codex-rs/core/src/exec_command/session_manager.rs new file mode 100644 index 0000000000..213b874bfa --- /dev/null +++ b/codex-rs/core/src/exec_command/session_manager.rs @@ -0,0 +1,677 @@ +use std::collections::HashMap; +use std::io::ErrorKind; +use std::io::Read; +use std::sync::Arc; +use std::sync::LazyLock; +use std::sync::Mutex as StdMutex; +use std::sync::atomic::AtomicU32; + +use portable_pty::CommandBuilder; +use portable_pty::PtySize; +use portable_pty::native_pty_system; +use tokio::sync::Mutex; +use tokio::sync::mpsc; +use tokio::sync::oneshot; +use tokio::time::Duration; +use tokio::time::Instant; +use tokio::time::timeout; + +use crate::exec_command::exec_command_params::ExecCommandParams; +use crate::exec_command::exec_command_params::WriteStdinParams; +use crate::exec_command::exec_command_session::ExecCommandSession; +use crate::exec_command::session_id::SessionId; +use codex_protocol::models::FunctionCallOutputPayload; + +pub static SESSION_MANAGER: LazyLock = LazyLock::new(SessionManager::default); + +#[derive(Debug, Default)] +pub struct SessionManager { + next_session_id: AtomicU32, + sessions: Mutex>, +} + +#[derive(Debug)] +pub struct ExecCommandOutput { + wall_time: Duration, + exit_status: ExitStatus, + original_token_count: Option, + output: String, +} + +impl ExecCommandOutput { + fn to_text_output(&self) -> String { + let wall_time_secs = self.wall_time.as_secs_f32(); + let termination_status = match self.exit_status { + ExitStatus::Exited(code) => format!("Process exited with code {code}"), + ExitStatus::Ongoing(session_id) => { + format!("Process running with session ID {}", session_id.0) + } + }; + let truncation_status = match self.original_token_count { + Some(tokens) => { + format!("\nWarning: truncated output (original token count: {tokens})") + } + None => "".to_string(), + }; + format!( + r#"Wall time: {wall_time_secs:.3} seconds +{termination_status}{truncation_status} +Output: +{output}"#, + output = self.output + ) + } +} + +#[derive(Debug)] +pub enum ExitStatus { + Exited(i32), + Ongoing(SessionId), +} + +pub fn result_into_payload(result: Result) -> FunctionCallOutputPayload { + match result { + Ok(output) => FunctionCallOutputPayload { + content: output.to_text_output(), + success: Some(true), + }, + Err(err) => FunctionCallOutputPayload { + content: err, + success: Some(false), + }, + } +} + +impl SessionManager { + /// Processes the request and is required to send a response via `outgoing`. + pub async fn handle_exec_command_request( + &self, + params: ExecCommandParams, + ) -> Result { + // Allocate a session id. + let session_id = SessionId( + self.next_session_id + .fetch_add(1, std::sync::atomic::Ordering::SeqCst), + ); + + let (session, mut exit_rx) = + create_exec_command_session(params.clone()) + .await + .map_err(|err| { + format!( + "failed to create exec command session for session id {}: {err}", + session_id.0 + ) + })?; + + // Insert into session map. + let mut output_rx = session.output_receiver(); + self.sessions.lock().await.insert(session_id, session); + + // Collect output until either timeout expires or process exits. + // Do not cap during collection; truncate at the end if needed. + // Use a modest initial capacity to avoid large preallocation. + let cap_bytes_u64 = params.max_output_tokens.saturating_mul(4); + let cap_bytes: usize = cap_bytes_u64.min(usize::MAX as u64) as usize; + let mut collected: Vec = Vec::with_capacity(4096); + + let start_time = Instant::now(); + let deadline = start_time + Duration::from_millis(params.yield_time_ms); + let mut exit_code: Option = None; + + loop { + if Instant::now() >= deadline { + break; + } + let remaining = deadline.saturating_duration_since(Instant::now()); + tokio::select! { + biased; + exit = &mut exit_rx => { + exit_code = exit.ok(); + // Small grace period to pull remaining buffered output + let grace_deadline = Instant::now() + Duration::from_millis(25); + while Instant::now() < grace_deadline { + match timeout(Duration::from_millis(1), output_rx.recv()).await { + Ok(Ok(chunk)) => { + collected.extend_from_slice(&chunk); + } + Ok(Err(tokio::sync::broadcast::error::RecvError::Lagged(_))) => { + // Skip missed messages; keep trying within grace period. + continue; + } + Ok(Err(tokio::sync::broadcast::error::RecvError::Closed)) => break, + Err(_) => break, + } + } + break; + } + chunk = timeout(remaining, output_rx.recv()) => { + match chunk { + Ok(Ok(chunk)) => { + collected.extend_from_slice(&chunk); + } + Ok(Err(tokio::sync::broadcast::error::RecvError::Lagged(_))) => { + // Skip missed messages; continue collecting fresh output. + } + Ok(Err(tokio::sync::broadcast::error::RecvError::Closed)) => { break; } + Err(_) => { break; } + } + } + } + } + + let output = String::from_utf8_lossy(&collected).to_string(); + + let exit_status = if let Some(code) = exit_code { + ExitStatus::Exited(code) + } else { + ExitStatus::Ongoing(session_id) + }; + + // If output exceeds cap, truncate the middle and record original token estimate. + let (output, original_token_count) = truncate_middle(&output, cap_bytes); + Ok(ExecCommandOutput { + wall_time: Instant::now().duration_since(start_time), + exit_status, + original_token_count, + output, + }) + } + + /// Write characters to a session's stdin and collect combined output for up to `yield_time_ms`. + pub async fn handle_write_stdin_request( + &self, + params: WriteStdinParams, + ) -> Result { + let WriteStdinParams { + session_id, + chars, + yield_time_ms, + max_output_tokens, + } = params; + + // Grab handles without holding the sessions lock across await points. + let (writer_tx, mut output_rx) = { + let sessions = self.sessions.lock().await; + match sessions.get(&session_id) { + Some(session) => (session.writer_sender(), session.output_receiver()), + None => { + return Err(format!("unknown session id {}", session_id.0)); + } + } + }; + + // Write stdin if provided. + if !chars.is_empty() && writer_tx.send(chars.into_bytes()).await.is_err() { + return Err("failed to write to stdin".to_string()); + } + + // Collect output up to yield_time_ms, truncating to max_output_tokens bytes. + let mut collected: Vec = Vec::with_capacity(4096); + let start_time = Instant::now(); + let deadline = start_time + Duration::from_millis(yield_time_ms); + loop { + let now = Instant::now(); + if now >= deadline { + break; + } + let remaining = deadline - now; + match timeout(remaining, output_rx.recv()).await { + Ok(Ok(chunk)) => { + // Collect all output within the time budget; truncate at the end. + collected.extend_from_slice(&chunk); + } + Ok(Err(tokio::sync::broadcast::error::RecvError::Lagged(_))) => { + // Skip missed messages; continue collecting fresh output. + } + Ok(Err(tokio::sync::broadcast::error::RecvError::Closed)) => break, + Err(_) => break, // timeout + } + } + + // Return structured output, truncating middle if over cap. + let output = String::from_utf8_lossy(&collected).to_string(); + let cap_bytes_u64 = max_output_tokens.saturating_mul(4); + let cap_bytes: usize = cap_bytes_u64.min(usize::MAX as u64) as usize; + let (output, original_token_count) = truncate_middle(&output, cap_bytes); + Ok(ExecCommandOutput { + wall_time: Instant::now().duration_since(start_time), + exit_status: ExitStatus::Ongoing(session_id), + original_token_count, + output, + }) + } +} + +/// Spawn PTY and child process per spawn_exec_command_session logic. +async fn create_exec_command_session( + params: ExecCommandParams, +) -> anyhow::Result<(ExecCommandSession, oneshot::Receiver)> { + let ExecCommandParams { + cmd, + yield_time_ms: _, + max_output_tokens: _, + shell, + login, + } = params; + + // Use the native pty implementation for the system + let pty_system = native_pty_system(); + + // Create a new pty + let pair = pty_system.openpty(PtySize { + rows: 24, + cols: 80, + pixel_width: 0, + pixel_height: 0, + })?; + + // Spawn a shell into the pty + let mut command_builder = CommandBuilder::new(shell); + let shell_mode_opt = if login { "-lc" } else { "-c" }; + command_builder.arg(shell_mode_opt); + command_builder.arg(cmd); + + let mut child = pair.slave.spawn_command(command_builder)?; + // Obtain a killer that can signal the process independently of `.wait()`. + let killer = child.clone_killer(); + + // Channel to forward write requests to the PTY writer. + let (writer_tx, mut writer_rx) = mpsc::channel::>(128); + // Broadcast for streaming PTY output to readers: subscribers receive from subscription time. + let (output_tx, _) = tokio::sync::broadcast::channel::>(256); + + // Reader task: drain PTY and forward chunks to output channel. + let mut reader = pair.master.try_clone_reader()?; + let output_tx_clone = output_tx.clone(); + let reader_handle = tokio::task::spawn_blocking(move || { + let mut buf = [0u8; 8192]; + loop { + match reader.read(&mut buf) { + Ok(0) => break, // EOF + Ok(n) => { + // Forward to broadcast; best-effort if there are subscribers. + let _ = output_tx_clone.send(buf[..n].to_vec()); + } + Err(ref e) if e.kind() == ErrorKind::Interrupted => { + // Retry on EINTR + continue; + } + Err(ref e) if e.kind() == ErrorKind::WouldBlock => { + // We're in a blocking thread; back off briefly and retry. + std::thread::sleep(Duration::from_millis(5)); + continue; + } + Err(_) => break, + } + } + }); + + // Writer task: apply stdin writes to the PTY writer. + let writer = pair.master.take_writer()?; + let writer = Arc::new(StdMutex::new(writer)); + let writer_handle = tokio::spawn({ + let writer = writer.clone(); + async move { + while let Some(bytes) = writer_rx.recv().await { + let writer = writer.clone(); + // Perform blocking write on a blocking thread. + let _ = tokio::task::spawn_blocking(move || { + if let Ok(mut guard) = writer.lock() { + use std::io::Write; + let _ = guard.write_all(&bytes); + let _ = guard.flush(); + } + }) + .await; + } + } + }); + + // Keep the child alive until it exits, then signal exit code. + let (exit_tx, exit_rx) = oneshot::channel::(); + let wait_handle = tokio::task::spawn_blocking(move || { + let code = match child.wait() { + Ok(status) => status.exit_code() as i32, + Err(_) => -1, + }; + let _ = exit_tx.send(code); + }); + + // Create and store the session with channels. + let session = ExecCommandSession::new( + writer_tx, + output_tx, + killer, + reader_handle, + writer_handle, + wait_handle, + ); + Ok((session, exit_rx)) +} + +/// Truncate the middle of a UTF-8 string to at most `max_bytes` bytes, +/// preserving the beginning and the end. Returns the possibly truncated +/// string and `Some(original_token_count)` (estimated at 4 bytes/token) +/// if truncation occurred; otherwise returns the original string and `None`. +fn truncate_middle(s: &str, max_bytes: usize) -> (String, Option) { + // No truncation needed + if s.len() <= max_bytes { + return (s.to_string(), None); + } + let est_tokens = (s.len() as u64).div_ceil(4); + if max_bytes == 0 { + // Cannot keep any content; still return a full marker (never truncated). + return ( + format!("…{} tokens truncated…", est_tokens), + Some(est_tokens), + ); + } + + // Helper to truncate a string to a given byte length on a char boundary. + fn truncate_on_boundary(input: &str, max_len: usize) -> &str { + if input.len() <= max_len { + return input; + } + let mut end = max_len; + while end > 0 && !input.is_char_boundary(end) { + end -= 1; + } + &input[..end] + } + + // Given a left/right budget, prefer newline boundaries; otherwise fall back + // to UTF-8 char boundaries. + fn pick_prefix_end(s: &str, left_budget: usize) -> usize { + if let Some(head) = s.get(..left_budget) + && let Some(i) = head.rfind('\n') + { + return i + 1; // keep the newline so suffix starts on a fresh line + } + truncate_on_boundary(s, left_budget).len() + } + + fn pick_suffix_start(s: &str, right_budget: usize) -> usize { + let start_tail = s.len().saturating_sub(right_budget); + if let Some(tail) = s.get(start_tail..) + && let Some(i) = tail.find('\n') + { + return start_tail + i + 1; // start after newline + } + // Fall back to a char boundary at or after start_tail. + let mut idx = start_tail.min(s.len()); + while idx < s.len() && !s.is_char_boundary(idx) { + idx += 1; + } + idx + } + + // Refine marker length and budgets until stable. Marker is never truncated. + let mut guess_tokens = est_tokens; // worst-case: everything truncated + for _ in 0..4 { + let marker = format!("…{} tokens truncated…", guess_tokens); + let marker_len = marker.len(); + let keep_budget = max_bytes.saturating_sub(marker_len); + if keep_budget == 0 { + // No room for any content within the cap; return a full, untruncated marker + // that reflects the entire truncated content. + return ( + format!("…{} tokens truncated…", est_tokens), + Some(est_tokens), + ); + } + + let left_budget = keep_budget / 2; + let right_budget = keep_budget - left_budget; + let prefix_end = pick_prefix_end(s, left_budget); + let mut suffix_start = pick_suffix_start(s, right_budget); + if suffix_start < prefix_end { + suffix_start = prefix_end; + } + let kept_content_bytes = prefix_end + (s.len() - suffix_start); + let truncated_content_bytes = s.len().saturating_sub(kept_content_bytes); + let new_tokens = (truncated_content_bytes as u64).div_ceil(4); + if new_tokens == guess_tokens { + let mut out = String::with_capacity(marker_len + kept_content_bytes + 1); + out.push_str(&s[..prefix_end]); + out.push_str(&marker); + // Place marker on its own line for symmetry when we keep line boundaries. + out.push('\n'); + out.push_str(&s[suffix_start..]); + return (out, Some(est_tokens)); + } + guess_tokens = new_tokens; + } + + // Fallback: use last guess to build output. + let marker = format!("…{} tokens truncated…", guess_tokens); + let marker_len = marker.len(); + let keep_budget = max_bytes.saturating_sub(marker_len); + if keep_budget == 0 { + return ( + format!("…{} tokens truncated…", est_tokens), + Some(est_tokens), + ); + } + let left_budget = keep_budget / 2; + let right_budget = keep_budget - left_budget; + let prefix_end = pick_prefix_end(s, left_budget); + let suffix_start = pick_suffix_start(s, right_budget); + let mut out = String::with_capacity(marker_len + prefix_end + (s.len() - suffix_start) + 1); + out.push_str(&s[..prefix_end]); + out.push_str(&marker); + out.push('\n'); + out.push_str(&s[suffix_start..]); + (out, Some(est_tokens)) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::exec_command::session_id::SessionId; + + /// Test that verifies that [`SessionManager::handle_exec_command_request()`] + /// and [`SessionManager::handle_write_stdin_request()`] work as expected + /// in the presence of a process that never terminates (but produces + /// output continuously). + #[cfg(unix)] + #[allow(clippy::print_stderr)] + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + async fn session_manager_streams_and_truncates_from_now() { + use crate::exec_command::exec_command_params::ExecCommandParams; + use crate::exec_command::exec_command_params::WriteStdinParams; + use tokio::time::sleep; + + let session_manager = SessionManager::default(); + // Long-running loop that prints an increasing counter every ~100ms. + // Use Python for a portable, reliable sleep across shells/PTYs. + let cmd = r#"python3 - <<'PY' +import sys, time +count = 0 +while True: + print(count) + sys.stdout.flush() + count += 100 + time.sleep(0.1) +PY"# + .to_string(); + + // Start the session and collect ~3s of output. + let params = ExecCommandParams { + cmd, + yield_time_ms: 3_000, + max_output_tokens: 1_000, // large enough to avoid truncation here + shell: "/bin/bash".to_string(), + login: false, + }; + let initial_output = match session_manager + .handle_exec_command_request(params.clone()) + .await + { + Ok(v) => v, + Err(e) => { + // PTY may be restricted in some sandboxes; skip in that case. + if e.contains("openpty") || e.contains("Operation not permitted") { + eprintln!("skipping test due to restricted PTY: {e}"); + return; + } + panic!("exec request failed unexpectedly: {e}"); + } + }; + eprintln!("initial output: {initial_output:?}"); + + // Should be ongoing (we launched a never-ending loop). + let session_id = match initial_output.exit_status { + ExitStatus::Ongoing(id) => id, + _ => panic!("expected ongoing session"), + }; + + // Parse the numeric lines and get the max observed value in the first window. + let first_nums = extract_monotonic_numbers(&initial_output.output); + assert!( + !first_nums.is_empty(), + "expected some output from first window" + ); + let first_max = *first_nums.iter().max().unwrap(); + + // Wait ~4s so counters progress while we're not reading. + sleep(Duration::from_millis(4_000)).await; + + // Now read ~3s of output "from now" only. + // Use a small token cap so truncation occurs and we test middle truncation. + let write_params = WriteStdinParams { + session_id, + chars: String::new(), + yield_time_ms: 3_000, + max_output_tokens: 16, // 16 tokens ~= 64 bytes -> likely truncation + }; + let second = session_manager + .handle_write_stdin_request(write_params) + .await + .expect("write stdin should succeed"); + + // Verify truncation metadata and size bound (cap is tokens*4 bytes). + assert!(second.original_token_count.is_some()); + let cap_bytes = (16u64 * 4) as usize; + assert!(second.output.len() <= cap_bytes); + // New middle marker should be present. + assert!( + second.output.contains("tokens truncated") && second.output.contains('…'), + "expected truncation marker in output, got: {}", + second.output + ); + + // Minimal freshness check: the earliest number we see in the second window + // should be significantly larger than the last from the first window. + let second_nums = extract_monotonic_numbers(&second.output); + assert!( + !second_nums.is_empty(), + "expected some numeric output from second window" + ); + let second_min = *second_nums.iter().min().unwrap(); + + // We slept 4 seconds (~40 ticks at 100ms/tick, each +100), so expect + // an increase of roughly 4000 or more. Allow a generous margin. + assert!( + second_min >= first_max + 2000, + "second_min={second_min} first_max={first_max}", + ); + } + + #[cfg(unix)] + fn extract_monotonic_numbers(s: &str) -> Vec { + s.lines() + .filter_map(|line| { + if !line.is_empty() + && line.chars().all(|c| c.is_ascii_digit()) + && let Ok(n) = line.parse::() + { + // Our generator increments by 100; ignore spurious fragments. + if n % 100 == 0 { + return Some(n); + } + } + None + }) + .collect() + } + + #[test] + fn to_text_output_exited_no_truncation() { + let out = ExecCommandOutput { + wall_time: Duration::from_millis(1234), + exit_status: ExitStatus::Exited(0), + original_token_count: None, + output: "hello".to_string(), + }; + let text = out.to_text_output(); + let expected = r#"Wall time: 1.234 seconds +Process exited with code 0 +Output: +hello"#; + assert_eq!(expected, text); + } + + #[test] + fn to_text_output_ongoing_with_truncation() { + let out = ExecCommandOutput { + wall_time: Duration::from_millis(500), + exit_status: ExitStatus::Ongoing(SessionId(42)), + original_token_count: Some(1000), + output: "abc".to_string(), + }; + let text = out.to_text_output(); + let expected = r#"Wall time: 0.500 seconds +Process running with session ID 42 +Warning: truncated output (original token count: 1000) +Output: +abc"#; + assert_eq!(expected, text); + } + + #[test] + fn truncate_middle_no_newlines_fallback() { + // A long string with no newlines that exceeds the cap. + let s = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + let max_bytes = 16; // force truncation + let (out, original) = truncate_middle(s, max_bytes); + // For very small caps, we return the full, untruncated marker, + // even if it exceeds the cap. + assert_eq!(out, "…16 tokens truncated…"); + // Original string length is 62 bytes => ceil(62/4) = 16 tokens. + assert_eq!(original, Some(16)); + } + + #[test] + fn truncate_middle_prefers_newline_boundaries() { + // Build a multi-line string of 20 numbered lines (each "NNN\n"). + let mut s = String::new(); + for i in 1..=20 { + s.push_str(&format!("{i:03}\n")); + } + // Total length: 20 lines * 4 bytes per line = 80 bytes. + assert_eq!(s.len(), 80); + + // Choose a cap that forces truncation while leaving room for + // a few lines on each side after accounting for the marker. + let max_bytes = 64; + // Expect exact output: first 4 lines, marker, last 4 lines, and correct token estimate (80/4 = 20). + assert_eq!( + truncate_middle(&s, max_bytes), + ( + r#"001 +002 +003 +004 +…12 tokens truncated… +017 +018 +019 +020 +"# + .to_string(), + Some(20) + ) + ); + } +} diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 6d4699bceb..ae18332087 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -20,6 +20,7 @@ mod conversation_history; mod environment_context; pub mod error; pub mod exec; +mod exec_command; pub mod exec_env; mod flags; pub mod git_info; diff --git a/codex-rs/core/src/openai_tools.rs b/codex-rs/core/src/openai_tools.rs index bb5e6dacbf..272c901dc2 100644 --- a/codex-rs/core/src/openai_tools.rs +++ b/codex-rs/core/src/openai_tools.rs @@ -56,6 +56,7 @@ pub enum ConfigShellToolType { DefaultShell, ShellWithRequest { sandbox_policy: SandboxPolicy }, LocalShell, + StreamableShell, } #[derive(Debug, Clone)] @@ -72,13 +73,16 @@ impl ToolsConfig { sandbox_policy: SandboxPolicy, include_plan_tool: bool, include_apply_patch_tool: bool, + use_streamable_shell_tool: bool, ) -> Self { - let mut shell_type = if model_family.uses_local_shell_tool { + let mut shell_type = if use_streamable_shell_tool { + ConfigShellToolType::StreamableShell + } else if model_family.uses_local_shell_tool { ConfigShellToolType::LocalShell } else { ConfigShellToolType::DefaultShell }; - if matches!(approval_policy, AskForApproval::OnRequest) { + if matches!(approval_policy, AskForApproval::OnRequest) && !use_streamable_shell_tool { shell_type = ConfigShellToolType::ShellWithRequest { sandbox_policy: sandbox_policy.clone(), } @@ -492,6 +496,14 @@ pub(crate) fn get_openai_tools( ConfigShellToolType::LocalShell => { tools.push(OpenAiTool::LocalShell {}); } + ConfigShellToolType::StreamableShell => { + tools.push(OpenAiTool::Function( + crate::exec_command::create_exec_command_tool_for_responses_api(), + )); + tools.push(OpenAiTool::Function( + crate::exec_command::create_write_stdin_tool_for_responses_api(), + )); + } } if config.plan_tool { @@ -564,6 +576,7 @@ mod tests { SandboxPolicy::ReadOnly, true, false, + /*use_experimental_streamable_shell_tool*/ false, ); let tools = get_openai_tools(&config, Some(HashMap::new())); @@ -579,6 +592,7 @@ mod tests { SandboxPolicy::ReadOnly, true, false, + /*use_experimental_streamable_shell_tool*/ false, ); let tools = get_openai_tools(&config, Some(HashMap::new())); @@ -594,6 +608,7 @@ mod tests { SandboxPolicy::ReadOnly, false, false, + /*use_experimental_streamable_shell_tool*/ false, ); let tools = get_openai_tools( &config, @@ -688,6 +703,7 @@ mod tests { SandboxPolicy::ReadOnly, false, false, + /*use_experimental_streamable_shell_tool*/ false, ); let tools = get_openai_tools( @@ -744,6 +760,7 @@ mod tests { SandboxPolicy::ReadOnly, false, false, + /*use_experimental_streamable_shell_tool*/ false, ); let tools = get_openai_tools( @@ -795,6 +812,7 @@ mod tests { SandboxPolicy::ReadOnly, false, false, + /*use_experimental_streamable_shell_tool*/ false, ); let tools = get_openai_tools( @@ -849,6 +867,7 @@ mod tests { SandboxPolicy::ReadOnly, false, false, + /*use_experimental_streamable_shell_tool*/ false, ); let tools = get_openai_tools( From 09819d9b4755855080dd64f208c5dc716690c06a Mon Sep 17 00:00:00 2001 From: easong-openai Date: Fri, 22 Aug 2025 20:34:43 -0700 Subject: [PATCH 14/16] Add the ability to interrupt and provide feedback to the model (#2381) --- codex-rs/tui/src/chatwidget.rs | 10 +++++++++- codex-rs/tui/src/user_approval_widget.rs | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 3f24a0176c..235280be9f 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -26,6 +26,7 @@ use codex_core::protocol::PatchApplyBeginEvent; use codex_core::protocol::StreamErrorEvent; use codex_core::protocol::TaskCompleteEvent; use codex_core::protocol::TokenUsage; +use codex_core::protocol::TurnAbortReason; use codex_core::protocol::TurnDiffEvent; use codex_protocol::parse_command::ParsedCommand; use crossterm::event::KeyEvent; @@ -820,7 +821,14 @@ impl ChatWidget { EventMsg::TaskComplete(TaskCompleteEvent { .. }) => self.on_task_complete(), EventMsg::TokenCount(token_usage) => self.on_token_count(token_usage), EventMsg::Error(ErrorEvent { message }) => self.on_error(message), - EventMsg::TurnAborted(_) => self.on_error("Turn interrupted".to_owned()), + EventMsg::TurnAborted(ev) => match ev.reason { + TurnAbortReason::Interrupted => { + self.on_error("Tell the model what to do differently".to_owned()) + } + TurnAbortReason::Replaced => { + self.on_error("Turn aborted: replaced by a new task".to_owned()) + } + }, EventMsg::PlanUpdate(update) => self.on_plan_update(update), EventMsg::ExecApprovalRequest(ev) => self.on_exec_approval_request(id, ev), EventMsg::ApplyPatchApprovalRequest(ev) => self.on_apply_patch_approval_request(id, ev), diff --git a/codex-rs/tui/src/user_approval_widget.rs b/codex-rs/tui/src/user_approval_widget.rs index de22f12b7b..69ee3524e8 100644 --- a/codex-rs/tui/src/user_approval_widget.rs +++ b/codex-rs/tui/src/user_approval_widget.rs @@ -74,6 +74,17 @@ static COMMAND_SELECT_OPTIONS: LazyLock> = LazyLock::new(|| { key: KeyCode::Char('n'), decision: ReviewDecision::Denied, }, + SelectOption { + label: Line::from(vec![ + "No, ".into(), + "provide ".into(), + "f".underlined(), + "eedback".into(), + ]), + description: "Do not run the command; provide feedback", + key: KeyCode::Char('f'), + decision: ReviewDecision::Abort, + }, ] }); @@ -91,6 +102,17 @@ static PATCH_SELECT_OPTIONS: LazyLock> = LazyLock::new(|| { key: KeyCode::Char('n'), decision: ReviewDecision::Denied, }, + SelectOption { + label: Line::from(vec![ + "No, ".into(), + "provide ".into(), + "f".underlined(), + "eedback".into(), + ]), + description: "Do not apply the changes; provide feedback", + key: KeyCode::Char('f'), + decision: ReviewDecision::Abort, + }, ] }); From eca97d85598b8d19cbe55fd2bf90c7499722fb84 Mon Sep 17 00:00:00 2001 From: easong-openai Date: Sat, 23 Aug 2025 01:06:22 -0700 Subject: [PATCH 15/16] transcript hint (#2605) Adds a hint to use ctrl-t to view transcript for more details image --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 2 ++ ...ttom_pane__chat_composer__tests__backspace_after_pastes.snap | 2 +- .../codex_tui__bottom_pane__chat_composer__tests__empty.snap | 2 +- .../codex_tui__bottom_pane__chat_composer__tests__large.snap | 2 +- ...tui__bottom_pane__chat_composer__tests__multiple_pastes.snap | 2 +- .../codex_tui__bottom_pane__chat_composer__tests__small.snap | 2 +- 6 files changed, 7 insertions(+), 5 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 986dfd6182..a6c6c1bb44 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -1130,6 +1130,8 @@ impl WidgetRef for &ChatComposer { Span::from(" send "), newline_hint_key.set_style(key_hint_style), Span::from(" newline "), + "Ctrl+T".set_style(key_hint_style), + Span::from(" transcript "), "Ctrl+C".set_style(key_hint_style), Span::from(" quit"), ] diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__backspace_after_pastes.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__backspace_after_pastes.snap index 178e2cc40f..6933fa6795 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__backspace_after_pastes.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__backspace_after_pastes.snap @@ -11,4 +11,4 @@ expression: terminal.backend() "▌ " "▌ " "▌ " -" ⏎ send Ctrl+J newline Ctrl+C quit " +" ⏎ send Ctrl+J newline Ctrl+T transcript Ctrl+C quit " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__empty.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__empty.snap index de227a3071..6d6d6b1a01 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__empty.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__empty.snap @@ -11,4 +11,4 @@ expression: terminal.backend() "▌ " "▌ " "▌ " -" ⏎ send Ctrl+J newline Ctrl+C quit " +" ⏎ send Ctrl+J newline Ctrl+T transcript Ctrl+C quit " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__large.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__large.snap index d3f67c9219..3539b5aaa0 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__large.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__large.snap @@ -11,4 +11,4 @@ expression: terminal.backend() "▌ " "▌ " "▌ " -" ⏎ send Ctrl+J newline Ctrl+C quit " +" ⏎ send Ctrl+J newline Ctrl+T transcript Ctrl+C quit " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__multiple_pastes.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__multiple_pastes.snap index 999e7b2e3f..a2827ff92d 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__multiple_pastes.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__multiple_pastes.snap @@ -11,4 +11,4 @@ expression: terminal.backend() "▌ " "▌ " "▌ " -" ⏎ send Ctrl+J newline Ctrl+C quit " +" ⏎ send Ctrl+J newline Ctrl+T transcript Ctrl+C quit " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__small.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__small.snap index 68d7711aab..b5e97f8b75 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__small.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__small.snap @@ -11,4 +11,4 @@ expression: terminal.backend() "▌ " "▌ " "▌ " -" ⏎ send Ctrl+J newline Ctrl+C quit " +" ⏎ send Ctrl+J newline Ctrl+T transcript Ctrl+C quit " From 957d44918d334e89473b9abad14df96f843a19aa Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Sat, 23 Aug 2025 09:54:31 -0700 Subject: [PATCH 16/16] send-aggregated output (#2364) We want to send an aggregated output of stderr and stdout so we don't have to aggregate it stderr+stdout as we lose order sometimes. --------- Co-authored-by: Gabriel Peal --- codex-rs/core/src/codex.rs | 207 ++++++++++++++++-- codex-rs/core/src/exec.rs | 85 +++---- codex-rs/core/tests/exec.rs | 8 +- codex-rs/core/tests/exec_stream_events.rs | 31 +++ .../src/event_processor_with_human_output.rs | 6 +- codex-rs/protocol/src/protocol.rs | 3 + codex-rs/tui/src/chatwidget/tests.rs | 4 + .../tests/fixtures/ideal-binary-response.txt | 4 +- 8 files changed, 276 insertions(+), 72 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index ec053ce4aa..a7f8e7c30d 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -147,6 +147,14 @@ pub struct CodexSpawnOk { } pub(crate) const INITIAL_SUBMIT_ID: &str = ""; +pub(crate) const SUBMISSION_CHANNEL_CAPACITY: usize = 64; + +// Model-formatting limits: clients get full streams; oonly content sent to the model is truncated. +pub(crate) const MODEL_FORMAT_MAX_BYTES: usize = 10 * 1024; // 10 KiB +pub(crate) const MODEL_FORMAT_MAX_LINES: usize = 256; // lines +pub(crate) const MODEL_FORMAT_HEAD_LINES: usize = MODEL_FORMAT_MAX_LINES / 2; +pub(crate) const MODEL_FORMAT_TAIL_LINES: usize = MODEL_FORMAT_MAX_LINES - MODEL_FORMAT_HEAD_LINES; // 128 +pub(crate) const MODEL_FORMAT_HEAD_BYTES: usize = MODEL_FORMAT_MAX_BYTES / 2; impl Codex { /// Spawn a new [`Codex`] and initialize the session. @@ -155,7 +163,7 @@ impl Codex { auth_manager: Arc, initial_history: Option>, ) -> CodexResult { - let (tx_sub, rx_sub) = async_channel::bounded(64); + let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); let (tx_event, rx_event) = async_channel::unbounded(); let user_instructions = get_user_instructions(&config).await; @@ -728,15 +736,15 @@ impl Session { let ExecToolCallOutput { stdout, stderr, + aggregated_output, duration, exit_code, } = output; - // Because stdout and stderr could each be up to 100 KiB, we send - // truncated versions. - const MAX_STREAM_OUTPUT: usize = 5 * 1024; // 5KiB - let stdout = stdout.text.chars().take(MAX_STREAM_OUTPUT).collect(); - let stderr = stderr.text.chars().take(MAX_STREAM_OUTPUT).collect(); + // Send full stdout/stderr to clients; do not truncate. + let stdout = stdout.text.clone(); + let stderr = stderr.text.clone(); let formatted_output = format_exec_output_str(output); + let aggregated_output: String = aggregated_output.text.clone(); let msg = if is_apply_patch { EventMsg::PatchApplyEnd(PatchApplyEndEvent { @@ -750,9 +758,10 @@ impl Session { call_id: call_id.to_string(), stdout, stderr, - formatted_output, - duration: *duration, + aggregated_output, exit_code: *exit_code, + duration: *duration, + formatted_output, }) }; @@ -810,6 +819,7 @@ impl Session { exit_code: -1, stdout: StreamOutput::new(String::new()), stderr: StreamOutput::new(get_error_message_ui(e)), + aggregated_output: StreamOutput::new(get_error_message_ui(e)), duration: Duration::default(), }; &output_stderr @@ -2604,23 +2614,103 @@ async fn handle_sandbox_error( fn format_exec_output_str(exec_output: &ExecToolCallOutput) -> String { let ExecToolCallOutput { - exit_code, - stdout, - stderr, - .. + aggregated_output, .. } = exec_output; - let is_success = *exit_code == 0; - let output = if is_success { stdout } else { stderr }; + // Head+tail truncation for the model: show the beginning and end with an elision. + // Clients still receive full streams; only this formatted summary is capped. - let mut formatted_output = output.text.clone(); - if let Some(truncated_after_lines) = output.truncated_after_lines { - formatted_output.push_str(&format!( - "\n\n[Output truncated after {truncated_after_lines} lines: too many lines or bytes.]", - )); + let s = aggregated_output.text.as_str(); + let total_lines = s.lines().count(); + if s.len() <= MODEL_FORMAT_MAX_BYTES && total_lines <= MODEL_FORMAT_MAX_LINES { + return s.to_string(); } - formatted_output + let lines: Vec<&str> = s.lines().collect(); + let head_take = MODEL_FORMAT_HEAD_LINES.min(lines.len()); + let tail_take = MODEL_FORMAT_TAIL_LINES.min(lines.len().saturating_sub(head_take)); + let omitted = lines.len().saturating_sub(head_take + tail_take); + + // Join head and tail blocks (lines() strips newlines; reinsert them) + let head_block = lines + .iter() + .take(head_take) + .cloned() + .collect::>() + .join("\n"); + let tail_block = if tail_take > 0 { + lines[lines.len() - tail_take..].join("\n") + } else { + String::new() + }; + let marker = format!("\n[... omitted {omitted} of {total_lines} lines ...]\n\n"); + + // Byte budgets for head/tail around the marker + let mut head_budget = MODEL_FORMAT_HEAD_BYTES.min(MODEL_FORMAT_MAX_BYTES); + let tail_budget = MODEL_FORMAT_MAX_BYTES.saturating_sub(head_budget + marker.len()); + if tail_budget == 0 && marker.len() >= MODEL_FORMAT_MAX_BYTES { + // Degenerate case: marker alone exceeds budget; return a clipped marker + return take_bytes_at_char_boundary(&marker, MODEL_FORMAT_MAX_BYTES).to_string(); + } + if tail_budget == 0 { + // Make room for the marker by shrinking head + head_budget = MODEL_FORMAT_MAX_BYTES.saturating_sub(marker.len()); + } + + // Enforce line-count cap by trimming head/tail lines + let head_lines_text = head_block; + let tail_lines_text = tail_block; + // Build final string respecting byte budgets + let head_part = take_bytes_at_char_boundary(&head_lines_text, head_budget); + let mut result = String::with_capacity(MODEL_FORMAT_MAX_BYTES.min(s.len())); + result.push_str(head_part); + result.push_str(&marker); + + let remaining = MODEL_FORMAT_MAX_BYTES.saturating_sub(result.len()); + let tail_budget_final = remaining; + let tail_part = take_last_bytes_at_char_boundary(&tail_lines_text, tail_budget_final); + result.push_str(tail_part); + + result +} + +// Truncate a &str to a byte budget at a char boundary (prefix) +#[inline] +fn take_bytes_at_char_boundary(s: &str, maxb: usize) -> &str { + if s.len() <= maxb { + return s; + } + let mut last_ok = 0; + for (i, ch) in s.char_indices() { + let nb = i + ch.len_utf8(); + if nb > maxb { + break; + } + last_ok = nb; + } + &s[..last_ok] +} + +// Take a suffix of a &str within a byte budget at a char boundary +#[inline] +fn take_last_bytes_at_char_boundary(s: &str, maxb: usize) -> &str { + if s.len() <= maxb { + return s; + } + let mut start = s.len(); + let mut used = 0usize; + for (i, ch) in s.char_indices().rev() { + let nb = ch.len_utf8(); + if used + nb > maxb { + break; + } + start = i; + used += nb; + if start == 0 { + break; + } + } + &s[start..] } /// Exec output is a pre-serialized JSON payload @@ -2771,6 +2861,7 @@ mod tests { use mcp_types::TextContent; use pretty_assertions::assert_eq; use serde_json::json; + use std::time::Duration as StdDuration; fn text_block(s: &str) -> ContentBlock { ContentBlock::TextContent(TextContent { @@ -2805,6 +2896,82 @@ mod tests { assert_eq!(expected, got); } + #[test] + fn model_truncation_head_tail_by_lines() { + // Build 400 short lines so line-count limit, not byte budget, triggers truncation + let lines: Vec = (1..=400).map(|i| format!("line{i}")).collect(); + let full = lines.join("\n"); + + let exec = ExecToolCallOutput { + exit_code: 0, + stdout: StreamOutput::new(String::new()), + stderr: StreamOutput::new(String::new()), + aggregated_output: StreamOutput::new(full.clone()), + duration: StdDuration::from_secs(1), + }; + + let out = format_exec_output_str(&exec); + + // Expect elision marker with correct counts + let omitted = 400 - MODEL_FORMAT_MAX_LINES; // 144 + let marker = format!("\n[... omitted {omitted} of 400 lines ...]\n\n"); + assert!(out.contains(&marker), "missing marker: {out}"); + + // Validate head and tail + let parts: Vec<&str> = out.split(&marker).collect(); + assert_eq!(parts.len(), 2, "expected one marker split"); + let head = parts[0]; + let tail = parts[1]; + + let expected_head: String = (1..=MODEL_FORMAT_HEAD_LINES) + .map(|i| format!("line{i}")) + .collect::>() + .join("\n"); + assert!(head.starts_with(&expected_head), "head mismatch"); + + let expected_tail: String = ((400 - MODEL_FORMAT_TAIL_LINES + 1)..=400) + .map(|i| format!("line{i}")) + .collect::>() + .join("\n"); + assert!(tail.ends_with(&expected_tail), "tail mismatch"); + } + + #[test] + fn model_truncation_respects_byte_budget() { + // Construct a large output (about 100kB) so byte budget dominates + let big_line = "x".repeat(100); + let full = std::iter::repeat_n(big_line.clone(), 1000) + .collect::>() + .join("\n"); + + let exec = ExecToolCallOutput { + exit_code: 0, + stdout: StreamOutput::new(String::new()), + stderr: StreamOutput::new(String::new()), + aggregated_output: StreamOutput::new(full.clone()), + duration: StdDuration::from_secs(1), + }; + + let out = format_exec_output_str(&exec); + assert!(out.len() <= MODEL_FORMAT_MAX_BYTES, "exceeds byte budget"); + assert!(out.contains("omitted"), "should contain elision marker"); + + // Ensure head and tail are drawn from the original + assert!(full.starts_with(out.chars().take(8).collect::().as_str())); + assert!( + full.ends_with( + out.chars() + .rev() + .take(8) + .collect::() + .chars() + .rev() + .collect::() + .as_str() + ) + ); + } + #[test] fn falls_back_to_content_when_structured_is_null() { let ctr = CallToolResult { diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 9430433c11..d74ec9fcd3 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -28,18 +28,17 @@ use crate::spawn::StdioPolicy; use crate::spawn::spawn_child_async; use serde_bytes::ByteBuf; -// Maximum we send for each stream, which is either: -// - 10KiB OR -// - 256 lines -const MAX_STREAM_OUTPUT: usize = 10 * 1024; -const MAX_STREAM_OUTPUT_LINES: usize = 256; - const DEFAULT_TIMEOUT_MS: u64 = 10_000; // Hardcode these since it does not seem worth including the libc crate just // for these. const SIGKILL_CODE: i32 = 9; const TIMEOUT_CODE: i32 = 64; +const EXIT_CODE_SIGNAL_BASE: i32 = 128; // conventional shell: 128 + signal + +// I/O buffer sizing +const READ_CHUNK_SIZE: usize = 8192; // bytes per read +const AGGREGATE_BUFFER_INITIAL_CAPACITY: usize = 8 * 1024; // 8 KiB #[derive(Debug, Clone)] pub struct ExecParams { @@ -153,6 +152,7 @@ pub async fn process_exec_tool_call( exit_code, stdout, stderr, + aggregated_output: raw_output.aggregated_output.from_utf8_lossy(), duration, }) } @@ -189,10 +189,11 @@ pub struct StreamOutput { pub truncated_after_lines: Option, } #[derive(Debug)] -pub struct RawExecToolCallOutput { +struct RawExecToolCallOutput { pub exit_status: ExitStatus, pub stdout: StreamOutput>, pub stderr: StreamOutput>, + pub aggregated_output: StreamOutput>, } impl StreamOutput { @@ -213,11 +214,17 @@ impl StreamOutput> { } } +#[inline] +fn append_all(dst: &mut Vec, src: &[u8]) { + dst.extend_from_slice(src); +} + #[derive(Debug)] pub struct ExecToolCallOutput { pub exit_code: i32, pub stdout: StreamOutput, pub stderr: StreamOutput, + pub aggregated_output: StreamOutput, pub duration: Duration, } @@ -253,7 +260,7 @@ async fn exec( /// Consumes the output of a child process, truncating it so it is suitable for /// use as the output of a `shell` tool call. Also enforces specified timeout. -pub(crate) async fn consume_truncated_output( +async fn consume_truncated_output( mut child: Child, timeout: Duration, stdout_stream: Option, @@ -273,19 +280,19 @@ pub(crate) async fn consume_truncated_output( )) })?; + let (agg_tx, agg_rx) = async_channel::unbounded::>(); + let stdout_handle = tokio::spawn(read_capped( BufReader::new(stdout_reader), - MAX_STREAM_OUTPUT, - MAX_STREAM_OUTPUT_LINES, stdout_stream.clone(), false, + Some(agg_tx.clone()), )); let stderr_handle = tokio::spawn(read_capped( BufReader::new(stderr_reader), - MAX_STREAM_OUTPUT, - MAX_STREAM_OUTPUT_LINES, stdout_stream.clone(), true, + Some(agg_tx.clone()), )); let exit_status = tokio::select! { @@ -297,38 +304,48 @@ pub(crate) async fn consume_truncated_output( // timeout child.start_kill()?; // Debatable whether `child.wait().await` should be called here. - synthetic_exit_status(128 + TIMEOUT_CODE) + synthetic_exit_status(EXIT_CODE_SIGNAL_BASE + TIMEOUT_CODE) } } } _ = tokio::signal::ctrl_c() => { child.start_kill()?; - synthetic_exit_status(128 + SIGKILL_CODE) + synthetic_exit_status(EXIT_CODE_SIGNAL_BASE + SIGKILL_CODE) } }; let stdout = stdout_handle.await??; let stderr = stderr_handle.await??; + drop(agg_tx); + + let mut combined_buf = Vec::with_capacity(AGGREGATE_BUFFER_INITIAL_CAPACITY); + while let Ok(chunk) = agg_rx.recv().await { + append_all(&mut combined_buf, &chunk); + } + let aggregated_output = StreamOutput { + text: combined_buf, + truncated_after_lines: None, + }; + Ok(RawExecToolCallOutput { exit_status, stdout, stderr, + aggregated_output, }) } async fn read_capped( mut reader: R, - max_output: usize, - max_lines: usize, stream: Option, is_stderr: bool, + aggregate_tx: Option>>, ) -> io::Result>> { - let mut buf = Vec::with_capacity(max_output.min(8 * 1024)); - let mut tmp = [0u8; 8192]; + let mut buf = Vec::with_capacity(AGGREGATE_BUFFER_INITIAL_CAPACITY); + let mut tmp = [0u8; READ_CHUNK_SIZE]; - let mut remaining_bytes = max_output; - let mut remaining_lines = max_lines; + // No caps: append all bytes loop { let n = reader.read(&mut tmp).await?; @@ -355,33 +372,17 @@ async fn read_capped( let _ = stream.tx_event.send(event).await; } - // Copy into the buffer only while we still have byte and line budget. - if remaining_bytes > 0 && remaining_lines > 0 { - let mut copy_len = 0; - for &b in &tmp[..n] { - if remaining_bytes == 0 || remaining_lines == 0 { - break; - } - copy_len += 1; - remaining_bytes -= 1; - if b == b'\n' { - remaining_lines -= 1; - } - } - buf.extend_from_slice(&tmp[..copy_len]); + if let Some(tx) = &aggregate_tx { + let _ = tx.send(tmp[..n].to_vec()).await; } - // Continue reading to EOF to avoid back-pressure, but discard once caps are hit. - } - let truncated = remaining_lines == 0 || remaining_bytes == 0; + append_all(&mut buf, &tmp[..n]); + // Continue reading to EOF to avoid back-pressure + } Ok(StreamOutput { text: buf, - truncated_after_lines: if truncated { - Some((max_lines - remaining_lines) as u32) - } else { - None - }, + truncated_after_lines: None, }) } diff --git a/codex-rs/core/tests/exec.rs b/codex-rs/core/tests/exec.rs index f011aa7a0a..9e0cffe67f 100644 --- a/codex-rs/core/tests/exec.rs +++ b/codex-rs/core/tests/exec.rs @@ -70,12 +70,12 @@ async fn truncates_output_lines() { let output = run_test_cmd(tmp, cmd).await.unwrap(); - let expected_output = (1..=256) + let expected_output = (1..=300) .map(|i| format!("{i}\n")) .collect::>() .join(""); assert_eq!(output.stdout.text, expected_output); - assert_eq!(output.stdout.truncated_after_lines, Some(256)); + assert_eq!(output.stdout.truncated_after_lines, None); } /// Command succeeds with exit code 0 normally @@ -91,8 +91,8 @@ async fn truncates_output_bytes() { let output = run_test_cmd(tmp, cmd).await.unwrap(); - assert_eq!(output.stdout.text.len(), 10240); - assert_eq!(output.stdout.truncated_after_lines, Some(10)); + assert!(output.stdout.text.len() >= 15000); + assert_eq!(output.stdout.truncated_after_lines, None); } /// Command not found returns exit code 127, this is not considered a sandbox error diff --git a/codex-rs/core/tests/exec_stream_events.rs b/codex-rs/core/tests/exec_stream_events.rs index 85d4eb3701..521823d2d4 100644 --- a/codex-rs/core/tests/exec_stream_events.rs +++ b/codex-rs/core/tests/exec_stream_events.rs @@ -139,3 +139,34 @@ async fn test_exec_stderr_stream_events_echo() { } assert_eq!(String::from_utf8_lossy(&err), "oops\n"); } + +#[tokio::test] +async fn test_aggregated_output_interleaves_in_order() { + // Spawn a shell that alternates stdout and stderr with sleeps to enforce order. + let cmd = vec![ + "/bin/sh".to_string(), + "-c".to_string(), + "printf 'O1\\n'; sleep 0.01; printf 'E1\\n' 1>&2; sleep 0.01; printf 'O2\\n'; sleep 0.01; printf 'E2\\n' 1>&2".to_string(), + ]; + + let params = ExecParams { + command: cmd, + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + timeout_ms: Some(5_000), + env: HashMap::new(), + with_escalated_permissions: None, + justification: None, + }; + + let policy = SandboxPolicy::new_read_only_policy(); + + let result = process_exec_tool_call(params, SandboxType::None, &policy, &None, None) + .await + .expect("process_exec_tool_call"); + + assert_eq!(result.exit_code, 0); + assert_eq!(result.stdout.text, "O1\nO2\n"); + assert_eq!(result.stderr.text, "E1\nE2\n"); + assert_eq!(result.aggregated_output.text, "O1\nE1\nO2\nE2\n"); + assert_eq!(result.aggregated_output.truncated_after_lines, None); +} diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 0f3b56b402..0f7e14ea40 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -287,8 +287,7 @@ impl EventProcessor for EventProcessorWithHumanOutput { EventMsg::ExecCommandOutputDelta(_) => {} EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id, - stdout, - stderr, + aggregated_output, duration, exit_code, .. @@ -304,8 +303,7 @@ impl EventProcessor for EventProcessorWithHumanOutput { ("".to_string(), format!("exec('{call_id}')")) }; - let output = if exit_code == 0 { stdout } else { stderr }; - let truncated_output = output + let truncated_output = aggregated_output .lines() .take(MAX_OUTPUT_LINES_FOR_EXEC_TOOL_CALL) .collect::>() diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index e803324a2d..7e7708b23c 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -685,6 +685,9 @@ pub struct ExecCommandEndEvent { pub stdout: String, /// Captured stderr pub stderr: String, + /// Captured aggregated output + #[serde(default)] + pub aggregated_output: String, /// The command's exit code. pub exit_code: i32, /// The duration of the command execution. diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 04dd3ad389..dfb80a69a9 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -263,6 +263,7 @@ fn exec_history_cell_shows_working_then_completed() { call_id: "call-1".into(), stdout: "done".into(), stderr: String::new(), + aggregated_output: "done".into(), exit_code: 0, duration: std::time::Duration::from_millis(5), formatted_output: "done".into(), @@ -313,6 +314,7 @@ fn exec_history_cell_shows_working_then_failed() { call_id: "call-2".into(), stdout: String::new(), stderr: "error".into(), + aggregated_output: "error".into(), exit_code: 2, duration: std::time::Duration::from_millis(7), formatted_output: "".into(), @@ -361,6 +363,7 @@ fn exec_history_extends_previous_when_consecutive() { call_id: "call-a".into(), stdout: "one".into(), stderr: String::new(), + aggregated_output: "one".into(), exit_code: 0, duration: std::time::Duration::from_millis(5), formatted_output: "one".into(), @@ -390,6 +393,7 @@ fn exec_history_extends_previous_when_consecutive() { call_id: "call-b".into(), stdout: "two".into(), stderr: String::new(), + aggregated_output: "two".into(), exit_code: 0, duration: std::time::Duration::from_millis(5), formatted_output: "two".into(), diff --git a/codex-rs/tui/tests/fixtures/ideal-binary-response.txt b/codex-rs/tui/tests/fixtures/ideal-binary-response.txt index 62d6af570f..56ed4a46a7 100644 --- a/codex-rs/tui/tests/fixtures/ideal-binary-response.txt +++ b/codex-rs/tui/tests/fixtures/ideal-binary-response.txt @@ -9,7 +9,7 @@ codex I’m going to scan the workspace and Cargo manifests to see build profiles and dependencies that impact binary size. Then I’ll summarize the main causes. ->_ +_ ✓ ls -la └ total 6696 drwxr-xr-x@ 39 easong staff 1248 Aug 9 08:49 . @@ -205,4 +205,4 @@ assertions—outputs are much larger than cargo build --release. If you want, I can outline targeted trims (e.g., strip = "debuginfo", opt-level = "z", panic abort, tighter tokio/reqwest features) and estimate impact per -binary. +binary. \ No newline at end of file