From c591e4b6cefe97d9d688332018cf637cb1e962ff Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 11 Nov 2025 11:42:39 +0000 Subject: [PATCH] V1 --- codex-rs/Cargo.lock | 10 +- codex-rs/core/src/tools/events.rs | 26 ++- .../core/src/tools/handlers/unified_exec.rs | 5 +- codex-rs/core/src/tools/parallel.rs | 4 +- codex-rs/core/src/unified_exec/mod.rs | 16 ++ .../core/src/unified_exec/session_manager.rs | 151 ++++++++++++++++-- codex-rs/tui/src/exec_cell/render.rs | 40 ++++- 7 files changed, 226 insertions(+), 26 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index cf5f961a62..8794f54af0 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4450,7 +4450,7 @@ checksum = "3af6b589e163c5a788fab00ce0c0366f6efbb9959c2f9874b224936af7fce7e1" dependencies = [ "base64", "indexmap 2.12.0", - "quick-xml", + "quick-xml 0.38.0", "serde", "time", ] @@ -7093,7 +7093,7 @@ version = "0.31.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c66a47e840dc20793f2264eb4b3e4ecb4b75d91c0dd4af04b456128e0bdd449d" dependencies = [ - "bitflags 2.9.1", + "bitflags 2.10.0", "rustix 1.0.8", "wayland-backend", "wayland-scanner", @@ -7105,7 +7105,7 @@ version = "0.32.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "efa790ed75fbfd71283bd2521a1cfdc022aabcc28bdcff00851f9e4ae88d9901" dependencies = [ - "bitflags 2.9.1", + "bitflags 2.10.0", "wayland-backend", "wayland-client", "wayland-scanner", @@ -7117,7 +7117,7 @@ version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "efd94963ed43cf9938a090ca4f7da58eb55325ec8200c3848963e98dc25b78ec" dependencies = [ - "bitflags 2.9.1", + "bitflags 2.10.0", "wayland-backend", "wayland-client", "wayland-protocols", @@ -7726,7 +7726,7 @@ dependencies = [ "os_pipe", "rustix 0.38.44", "tempfile", - "thiserror 2.0.16", + "thiserror 2.0.17", "tree_magic_mini", "wayland-backend", "wayland-client", diff --git a/codex-rs/core/src/tools/events.rs b/codex-rs/core/src/tools/events.rs index 9019dac5ed..785f5d4834 100644 --- a/codex-rs/core/src/tools/events.rs +++ b/codex-rs/core/src/tools/events.rs @@ -88,6 +88,7 @@ pub(crate) enum ToolEmitter { }, UnifiedExec { command: String, + display_command: Vec, cwd: PathBuf, // True for `exec_command` and false for `write_stdin`. #[allow(dead_code)] @@ -111,9 +112,15 @@ impl ToolEmitter { } } - pub fn unified_exec(command: String, cwd: PathBuf, is_startup_command: bool) -> Self { + pub fn unified_exec( + command: String, + display_command: Vec, + cwd: PathBuf, + is_startup_command: bool, + ) -> Self { Self::UnifiedExec { command, + display_command, cwd, is_startup_command, } @@ -217,8 +224,21 @@ impl ToolEmitter { ) => { emit_patch_end(ctx, String::new(), (*message).to_string(), false).await; } - (Self::UnifiedExec { command, cwd, .. }, ToolEventStage::Begin) => { - emit_exec_command_begin(ctx, &[command.to_string()], cwd.as_path(), false).await; + ( + Self::UnifiedExec { + command, + display_command, + cwd, + .. + }, + ToolEventStage::Begin, + ) => { + let command_args = if display_command.is_empty() { + vec![command.clone()] + } else { + display_command.clone() + }; + emit_exec_command_begin(ctx, &command_args, cwd.as_path(), false).await; } (Self::UnifiedExec { .. }, ToolEventStage::Success(output)) => { emit_exec_end( diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 6542c333cc..6c1f42486a 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -20,6 +20,7 @@ use crate::unified_exec::UnifiedExecContext; use crate::unified_exec::UnifiedExecResponse; use crate::unified_exec::UnifiedExecSessionManager; use crate::unified_exec::WriteStdinRequest; +use crate::unified_exec::build_shell_command; pub struct UnifiedExecHandler; @@ -113,7 +114,9 @@ impl ToolHandler for UnifiedExecHandler { &context.call_id, None, ); - let emitter = ToolEmitter::unified_exec(args.cmd.clone(), cwd.clone(), true); + let display_command = build_shell_command(&args.shell, args.login, &args.cmd); + let emitter = + ToolEmitter::unified_exec(args.cmd.clone(), display_command, cwd.clone(), true); emitter.emit(event_ctx, ToolEventStage::Begin).await; manager diff --git a/codex-rs/core/src/tools/parallel.rs b/codex-rs/core/src/tools/parallel.rs index baa5321da8..5388ab9dac 100644 --- a/codex-rs/core/src/tools/parallel.rs +++ b/codex-rs/core/src/tools/parallel.rs @@ -65,9 +65,9 @@ impl ToolCallRuntime { Ok(Self::aborted_response(&call, secs)) }, res = async { - tracing::info!("waiting for tool gate"); + tracing::trace!("waiting for tool gate"); readiness.wait_ready().await; - tracing::info!("tool gate released"); + tracing::trace!("tool gate released"); let _guard = if supports_parallel { Either::Left(lock.read().await) } else { diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 16fbc4c7f0..c7ed0517e6 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -92,18 +92,34 @@ pub(crate) struct UnifiedExecResponse { pub original_token_count: Option, } +pub(crate) fn build_shell_command(shell: &str, login: bool, command: &str) -> Vec { + let shell_flag = if login { "-lc" } else { "-c" }; + vec![ + shell.to_string(), + shell_flag.to_string(), + command.to_string(), + ] +} + #[derive(Default)] pub(crate) struct UnifiedExecSessionManager { next_session_id: AtomicI32, sessions: Mutex>, } +#[derive(Clone, Debug)] +pub(crate) struct StdinEvent { + pub input: String, + pub output: String, +} + struct SessionEntry { session: session::UnifiedExecSession, session_ref: Arc, turn_ref: Arc, call_id: String, command: String, + stdin_events: Vec, cwd: PathBuf, started_at: tokio::time::Instant, } diff --git a/codex-rs/core/src/unified_exec/session_manager.rs b/codex-rs/core/src/unified_exec/session_manager.rs index 55a5f589a3..03b0876bed 100644 --- a/codex-rs/core/src/unified_exec/session_manager.rs +++ b/codex-rs/core/src/unified_exec/session_manager.rs @@ -21,11 +21,13 @@ use crate::tools::sandboxing::ToolCtx; use super::ExecCommandRequest; use super::MIN_YIELD_TIME_MS; use super::SessionEntry; +use super::StdinEvent; use super::UnifiedExecContext; use super::UnifiedExecError; use super::UnifiedExecResponse; use super::UnifiedExecSessionManager; use super::WriteStdinRequest; +use super::build_shell_command; use super::clamp_yield_time; use super::generate_chunk_id; use super::resolve_max_tokens; @@ -33,6 +35,33 @@ use super::session::OutputBuffer; use super::session::UnifiedExecSession; use super::truncate_output_to_tokens; +fn escape_stdin_for_log(input: &str) -> String { + let mut escaped = String::with_capacity(input.len()); + for ch in input.chars() { + match ch { + '\n' => escaped.push_str("\\n"), + '\r' => escaped.push_str("\\r"), + '\t' => escaped.push_str("\\t"), + '\x1b' => escaped.push_str("\\u001b"), + ctrl if ctrl.is_control() => { + let code = u32::from(ctrl); + escaped.push_str(&format!("\\u{code:04x}")); + } + other => escaped.push(other), + } + } + escaped +} + +fn render_stdin_events(events: &[StdinEvent]) -> String { + let mut lines = Vec::new(); + for event in events { + lines.push(event.input.clone()); + lines.push(format!("> {}", event.output)); + } + lines.join("\n") +} + impl UnifiedExecSessionManager { pub(crate) async fn exec_command( &self, @@ -43,15 +72,11 @@ impl UnifiedExecSessionManager { .workdir .clone() .unwrap_or_else(|| context.turn.cwd.clone()); - let shell_flag = if request.login { "-lc" } else { "-c" }; - let command = vec![ - request.shell.to_string(), - shell_flag.to_string(), - request.command.to_string(), - ]; + let command_argv = build_shell_command(request.shell, request.login, request.command); + let command_line = request.command.to_string(); let session = self - .open_session_with_sandbox(command, cwd.clone(), context) + .open_session_with_sandbox(command_argv.clone(), cwd.clone(), context) .await?; let max_tokens = resolve_max_tokens(request.max_output_tokens); @@ -73,7 +98,7 @@ impl UnifiedExecSessionManager { None } else { Some( - self.store_session(session, context, request.command, cwd.clone(), start) + self.store_session(session, context, &command_line, cwd.clone(), start) .await, ) }; @@ -93,7 +118,7 @@ impl UnifiedExecSessionManager { let exit = response.exit_code.unwrap_or(-1); Self::emit_exec_end_from_context( context, - request.command.to_string(), + command_line, cwd, response.output.clone(), exit, @@ -114,6 +139,12 @@ impl UnifiedExecSessionManager { let (writer_tx, output_buffer, output_notify) = self.prepare_session_handles(session_id).await?; + let logged_input = if request.input.is_empty() { + None + } else { + Some(escape_stdin_for_log(request.input)) + }; + if !request.input.is_empty() { Self::send_input(&writer_tx, request.input.as_bytes()).await?; tokio::time::sleep(Duration::from_millis(100)).await; @@ -128,11 +159,36 @@ impl UnifiedExecSessionManager { let wall_time = Instant::now().saturating_duration_since(start); let text = String::from_utf8_lossy(&collected).to_string(); - let (output, original_token_count) = truncate_output_to_tokens(&text, max_tokens); + let escaped_output = escape_stdin_for_log(&text); + let output_for_event = if escaped_output.is_empty() + || logged_input.as_ref().is_some_and(|input| { + input.trim_end_matches("\\n") == escaped_output.trim_end_matches("\\n") + }) { + "[no output]".to_string() + } else { + escaped_output.clone() + }; + + let stdin_events = if let Some(input) = logged_input { + self.append_stdin_event( + session_id, + StdinEvent { + input, + output: output_for_event, + }, + ) + .await? + } else { + self.clone_stdin_events(session_id).await? + }; + + let aggregated_text = render_stdin_events(&stdin_events); + let (output, original_token_count) = + truncate_output_to_tokens(&aggregated_text, max_tokens); let chunk_id = generate_chunk_id(); let status = self.refresh_session_state(session_id).await; - let (session_id, exit_code, completion_entry, event_call_id) = match status { + let (session_id, exit_code, mut completion_entry, event_call_id) = match status { SessionStatus::Alive { exit_code, call_id } => { (Some(session_id), exit_code, None, call_id) } @@ -155,7 +211,7 @@ impl UnifiedExecSessionManager { original_token_count, }; - if let (Some(exit), Some(entry)) = (response.exit_code, completion_entry) { + if let (Some(exit), Some(entry)) = (response.exit_code, completion_entry.take()) { let total_duration = Instant::now().saturating_duration_since(entry.started_at); Self::emit_exec_end_from_entry(entry, response.output.clone(), exit, total_duration) .await; @@ -214,6 +270,30 @@ impl UnifiedExecSessionManager { .map_err(|_| UnifiedExecError::WriteToStdin) } + async fn append_stdin_event( + &self, + session_id: i32, + event: StdinEvent, + ) -> Result, UnifiedExecError> { + let mut sessions = self.sessions.lock().await; + let Some(entry) = sessions.get_mut(&session_id) else { + return Err(UnifiedExecError::UnknownSessionId { session_id }); + }; + entry.stdin_events.push(event); + Ok(entry.stdin_events.clone()) + } + + async fn clone_stdin_events( + &self, + session_id: i32, + ) -> Result, UnifiedExecError> { + let sessions = self.sessions.lock().await; + let Some(entry) = sessions.get(&session_id) else { + return Err(UnifiedExecError::UnknownSessionId { session_id }); + }; + Ok(entry.stdin_events.clone()) + } + async fn store_session( &self, session: UnifiedExecSession, @@ -231,6 +311,7 @@ impl UnifiedExecSessionManager { turn_ref: Arc::clone(&context.turn), call_id: context.call_id.clone(), command: command.to_string(), + stdin_events: Vec::new(), cwd, started_at, }; @@ -258,7 +339,8 @@ impl UnifiedExecSessionManager { &entry.call_id, None, ); - let emitter = ToolEmitter::unified_exec(entry.command, entry.cwd, true); + let emitter = + ToolEmitter::unified_exec(entry.command.clone(), vec![entry.command], entry.cwd, true); emitter .emit(event_ctx, ToolEventStage::Success(output)) .await; @@ -286,7 +368,7 @@ impl UnifiedExecSessionManager { &context.call_id, None, ); - let emitter = ToolEmitter::unified_exec(command, cwd, true); + let emitter = ToolEmitter::unified_exec(command.clone(), vec![command], cwd, true); emitter .emit(event_ctx, ToolEventStage::Success(output)) .await; @@ -400,3 +482,44 @@ enum SessionStatus { }, Unknown, } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn escape_stdin_for_log_escapes_control_chars() { + assert_eq!(escape_stdin_for_log("a\n\t\x1b"), "a\\n\\t\\u001b"); + } + + #[test] + fn render_stdin_events_keeps_order() { + let events = vec![ + StdinEvent { + input: "first".to_string(), + output: "[no output]".to_string(), + }, + StdinEvent { + input: "second".to_string(), + output: "out".to_string(), + }, + ]; + + let rendered = render_stdin_events(&events); + assert_eq!(rendered, "first\n> [no output]\nsecond\n> out"); + } + + #[test] + fn transcript_is_truncated_after_merge() { + let rendered = render_stdin_events(&[StdinEvent { + input: "abcde".to_string(), + output: "output".to_string(), + }]); + let expected_prefix: String = rendered.chars().take(6).collect(); + let (truncated, original_len) = truncate_output_to_tokens(&rendered, 6); + + assert_eq!(truncated, expected_prefix); + assert_eq!(original_len, Some(rendered.chars().count())); + } +} diff --git a/codex-rs/tui/src/exec_cell/render.rs b/codex-rs/tui/src/exec_cell/render.rs index 8ebc125125..4e44f43de6 100644 --- a/codex-rs/tui/src/exec_cell/render.rs +++ b/codex-rs/tui/src/exec_cell/render.rs @@ -23,7 +23,7 @@ use textwrap::WordSplitter; use unicode_width::UnicodeWidthStr; pub(crate) const TOOL_CALL_MAX_LINES: usize = 5; -const USER_SHELL_TOOL_CALL_MAX_LINES: usize = 50; +const USER_SHELL_TOOL_CALL_MAX_LINES: usize = usize::MAX; pub(crate) struct OutputLinesParams { pub(crate) line_limit: usize, @@ -524,6 +524,44 @@ struct ExecDisplayLayout { output_max_lines: usize, } +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn user_shell_output_shows_all_lines() { + let aggregated_output = (0..12) + .map(|idx| format!("line {idx}")) + .collect::>() + .join("\n"); + let command_output = CommandOutput { + exit_code: 0, + aggregated_output, + formatted_output: String::new(), + }; + + let output = output_lines( + Some(&command_output), + OutputLinesParams { + line_limit: USER_SHELL_TOOL_CALL_MAX_LINES, + only_err: false, + include_angle_pipe: true, + include_prefix: true, + }, + ); + + assert_eq!(output.lines.len(), 12); + assert!(output.omitted.is_none()); + assert!( + output + .lines + .iter() + .all(|line| line.spans.iter().all(|span| !span.content.contains('…'))) + ); + } +} + impl ExecDisplayLayout { const fn new( command_continuation: PrefixedBlock,