From 1f3318c1c5b83e29b6d6e61eb2bf7d599581023e Mon Sep 17 00:00:00 2001 From: Gabriel Peal Date: Mon, 4 Aug 2025 08:57:04 -0700 Subject: [PATCH 01/12] Add a TurnDiffTracker to create a unified diff for an entire turn (#1770) This lets us show an accumulating diff across all patches in a turn. Refer to the docs for TurnDiffTracker for implementation details. There are multiple ways this could have been done and this felt like the right tradeoff between reliability and completeness: *Pros* * It will pick up all changes to files that the model touched including if they prettier or another command that updates them. * It will not pick up changes made by the user or other agents to files it didn't modify. *Cons* * It will pick up changes that the user made to a file that the model also touched * It will not pick up changes to codegen or files that were not modified with apply_patch --- codex-rs/Cargo.lock | 1 + codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/codex.rs | 111 ++- codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/protocol.rs | 7 + codex-rs/core/src/turn_diff_tracker.rs | 887 ++++++++++++++++++ .../src/event_processor_with_human_output.rs | 6 + codex-rs/mcp-server/src/codex_tool_runner.rs | 1 + codex-rs/mcp-server/src/conversation_loop.rs | 1 + 9 files changed, 998 insertions(+), 18 deletions(-) create mode 100644 codex-rs/core/src/turn_diff_tracker.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 7d4e41d0b1..eb4eccd897 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -699,6 +699,7 @@ dependencies = [ "serde_json", "sha1", "shlex", + "similar", "strum_macros 0.27.2", "tempfile", "thiserror 2.0.12", diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index db3fd4f834..466e9adf02 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -34,6 +34,7 @@ serde_json = "1" serde_bytes = "0.11" sha1 = "0.10.6" shlex = "1.3.0" +similar = "2.7.0" strum_macros = "0.27.2" thiserror = "2.0.12" time = { version = "0.3", features = ["formatting", "local-offset", "macros"] } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 7004dcfcb7..568d87c4a8 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -85,11 +85,13 @@ use crate::protocol::SandboxPolicy; use crate::protocol::SessionConfiguredEvent; use crate::protocol::Submission; use crate::protocol::TaskCompleteEvent; +use crate::protocol::TurnDiffEvent; use crate::rollout::RolloutRecorder; use crate::safety::SafetyCheck; use crate::safety::assess_command_safety; use crate::safety::assess_safety_for_untrusted_command; use crate::shell; +use crate::turn_diff_tracker::TurnDiffTracker; use crate::user_notification::UserNotification; use crate::util::backoff; @@ -362,7 +364,11 @@ impl Session { } } - async fn notify_exec_command_begin(&self, exec_command_context: ExecCommandContext) { + async fn on_exec_command_begin( + &self, + turn_diff_tracker: &mut TurnDiffTracker, + exec_command_context: ExecCommandContext, + ) { let ExecCommandContext { sub_id, call_id, @@ -374,11 +380,15 @@ impl Session { Some(ApplyPatchCommandContext { user_explicitly_approved_this_action, changes, - }) => EventMsg::PatchApplyBegin(PatchApplyBeginEvent { - call_id, - auto_approved: !user_explicitly_approved_this_action, - changes, - }), + }) => { + turn_diff_tracker.on_patch_begin(&changes); + + EventMsg::PatchApplyBegin(PatchApplyBeginEvent { + call_id, + auto_approved: !user_explicitly_approved_this_action, + changes, + }) + } None => EventMsg::ExecCommandBegin(ExecCommandBeginEvent { call_id, command: command_for_display.clone(), @@ -392,8 +402,10 @@ impl Session { let _ = self.tx_event.send(event).await; } - async fn notify_exec_command_end( + #[allow(clippy::too_many_arguments)] + async fn on_exec_command_end( &self, + turn_diff_tracker: &mut TurnDiffTracker, sub_id: &str, call_id: &str, output: &ExecToolCallOutput, @@ -433,6 +445,20 @@ impl Session { msg, }; let _ = self.tx_event.send(event).await; + + // If this is an apply_patch, after we emit the end patch, emit a second event + // with the full turn diff if there is one. + if is_apply_patch { + let unified_diff = turn_diff_tracker.get_unified_diff(); + if let Ok(Some(unified_diff)) = unified_diff { + let msg = EventMsg::TurnDiff(TurnDiffEvent { unified_diff }); + let event = Event { + id: sub_id.into(), + msg, + }; + let _ = self.tx_event.send(event).await; + } + } } /// Helper that emits a BackgroundEvent with the given message. This keeps @@ -1006,6 +1032,10 @@ async fn run_task(sess: Arc, sub_id: String, input: Vec) { .await; let last_agent_message: Option; + // Although from the perspective of codex.rs, TurnDiffTracker has the lifecycle of a Task which contains + // many turns, from the perspective of the user, it is a single turn. + let mut turn_diff_tracker = TurnDiffTracker::new(); + loop { // Note that pending_input would be something like a message the user // submitted through the UI while the model was running. Though the UI @@ -1037,7 +1067,7 @@ async fn run_task(sess: Arc, sub_id: String, input: Vec) { }) }) .collect(); - match run_turn(&sess, sub_id.clone(), turn_input).await { + match run_turn(&sess, &mut turn_diff_tracker, sub_id.clone(), turn_input).await { Ok(turn_output) => { let mut items_to_record_in_conversation_history = Vec::::new(); let mut responses = Vec::::new(); @@ -1163,6 +1193,7 @@ async fn run_task(sess: Arc, sub_id: String, input: Vec) { async fn run_turn( sess: &Session, + turn_diff_tracker: &mut TurnDiffTracker, sub_id: String, input: Vec, ) -> CodexResult> { @@ -1177,7 +1208,7 @@ async fn run_turn( let mut retries = 0; loop { - match try_run_turn(sess, &sub_id, &prompt).await { + match try_run_turn(sess, turn_diff_tracker, &sub_id, &prompt).await { Ok(output) => return Ok(output), Err(CodexErr::Interrupted) => return Err(CodexErr::Interrupted), Err(CodexErr::EnvVar(var)) => return Err(CodexErr::EnvVar(var)), @@ -1223,6 +1254,7 @@ struct ProcessedResponseItem { async fn try_run_turn( sess: &Session, + turn_diff_tracker: &mut TurnDiffTracker, sub_id: &str, prompt: &Prompt, ) -> CodexResult> { @@ -1310,7 +1342,8 @@ async fn try_run_turn( match event { ResponseEvent::Created => {} ResponseEvent::OutputItemDone(item) => { - let response = handle_response_item(sess, sub_id, item.clone()).await?; + let response = + handle_response_item(sess, turn_diff_tracker, sub_id, item.clone()).await?; output.push(ProcessedResponseItem { item, response }); } @@ -1328,6 +1361,16 @@ async fn try_run_turn( .ok(); } + let unified_diff = turn_diff_tracker.get_unified_diff(); + if let Ok(Some(unified_diff)) = unified_diff { + let msg = EventMsg::TurnDiff(TurnDiffEvent { unified_diff }); + let event = Event { + id: sub_id.to_string(), + msg, + }; + let _ = sess.tx_event.send(event).await; + } + return Ok(output); } ResponseEvent::OutputTextDelta(delta) => { @@ -1432,6 +1475,7 @@ async fn run_compact_task( async fn handle_response_item( sess: &Session, + turn_diff_tracker: &mut TurnDiffTracker, sub_id: &str, item: ResponseItem, ) -> CodexResult> { @@ -1469,7 +1513,17 @@ async fn handle_response_item( .. } => { info!("FunctionCall: {arguments}"); - Some(handle_function_call(sess, sub_id.to_string(), name, arguments, call_id).await) + Some( + handle_function_call( + sess, + turn_diff_tracker, + sub_id.to_string(), + name, + arguments, + call_id, + ) + .await, + ) } ResponseItem::LocalShellCall { id, @@ -1504,6 +1558,7 @@ async fn handle_response_item( handle_container_exec_with_params( exec_params, sess, + turn_diff_tracker, sub_id.to_string(), effective_call_id, ) @@ -1521,6 +1576,7 @@ async fn handle_response_item( async fn handle_function_call( sess: &Session, + turn_diff_tracker: &mut TurnDiffTracker, sub_id: String, name: String, arguments: String, @@ -1534,7 +1590,8 @@ async fn handle_function_call( return *output; } }; - handle_container_exec_with_params(params, sess, sub_id, call_id).await + handle_container_exec_with_params(params, sess, turn_diff_tracker, sub_id, call_id) + .await } "update_plan" => handle_update_plan(sess, arguments, sub_id, call_id).await, _ => { @@ -1608,6 +1665,7 @@ fn maybe_run_with_user_profile(params: ExecParams, sess: &Session) -> ExecParams async fn handle_container_exec_with_params( params: ExecParams, sess: &Session, + turn_diff_tracker: &mut TurnDiffTracker, sub_id: String, call_id: String, ) -> ResponseInputItem { @@ -1755,7 +1813,7 @@ async fn handle_container_exec_with_params( }, ), }; - sess.notify_exec_command_begin(exec_command_context.clone()) + sess.on_exec_command_begin(turn_diff_tracker, exec_command_context.clone()) .await; let params = maybe_run_with_user_profile(params, sess); @@ -1782,7 +1840,8 @@ async fn handle_container_exec_with_params( duration, } = &output; - sess.notify_exec_command_end( + sess.on_exec_command_end( + turn_diff_tracker, &sub_id, &call_id, &output, @@ -1806,7 +1865,15 @@ async fn handle_container_exec_with_params( } } Err(CodexErr::Sandbox(error)) => { - handle_sandbox_error(params, exec_command_context, error, sandbox_type, sess).await + handle_sandbox_error( + turn_diff_tracker, + params, + exec_command_context, + error, + sandbox_type, + sess, + ) + .await } Err(e) => { // Handle non-sandbox errors @@ -1822,6 +1889,7 @@ async fn handle_container_exec_with_params( } async fn handle_sandbox_error( + turn_diff_tracker: &mut TurnDiffTracker, params: ExecParams, exec_command_context: ExecCommandContext, error: SandboxErr, @@ -1878,7 +1946,8 @@ async fn handle_sandbox_error( sess.notify_background_event(&sub_id, "retrying command without sandbox") .await; - sess.notify_exec_command_begin(exec_command_context).await; + sess.on_exec_command_begin(turn_diff_tracker, exec_command_context) + .await; // This is an escalated retry; the policy will not be // examined and the sandbox has been set to `None`. @@ -1905,8 +1974,14 @@ async fn handle_sandbox_error( duration, } = &retry_output; - sess.notify_exec_command_end(&sub_id, &call_id, &retry_output, is_apply_patch) - .await; + sess.on_exec_command_end( + turn_diff_tracker, + &sub_id, + &call_id, + &retry_output, + is_apply_patch, + ) + .await; let is_success = *exit_code == 0; let content = format_exec_output( diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 80f9014954..4f083d9e56 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -42,6 +42,7 @@ pub(crate) mod safety; pub mod seatbelt; pub mod shell; pub mod spawn; +pub mod turn_diff_tracker; mod user_notification; pub mod util; diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index cbb211d955..82591a2c78 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -387,6 +387,8 @@ pub enum EventMsg { /// Notification that a patch application has finished. PatchApplyEnd(PatchApplyEndEvent), + TurnDiff(TurnDiffEvent), + /// Response to GetHistoryEntryRequest. GetHistoryEntryResponse(GetHistoryEntryResponseEvent), @@ -598,6 +600,11 @@ pub struct PatchApplyEndEvent { pub success: bool, } +#[derive(Debug, Clone, Deserialize, Serialize)] +pub struct TurnDiffEvent { + pub unified_diff: String, +} + #[derive(Debug, Clone, Deserialize, Serialize)] pub struct GetHistoryEntryResponseEvent { pub offset: usize, diff --git a/codex-rs/core/src/turn_diff_tracker.rs b/codex-rs/core/src/turn_diff_tracker.rs new file mode 100644 index 0000000000..7026d7bb32 --- /dev/null +++ b/codex-rs/core/src/turn_diff_tracker.rs @@ -0,0 +1,887 @@ +use std::collections::HashMap; +use std::fs; +use std::path::Path; +use std::path::PathBuf; +use std::process::Command; + +use anyhow::Context; +use anyhow::Result; +use anyhow::anyhow; +use sha1::digest::Output; +use uuid::Uuid; + +use crate::protocol::FileChange; + +const ZERO_OID: &str = "0000000000000000000000000000000000000000"; +const DEV_NULL: &str = "/dev/null"; + +struct BaselineFileInfo { + path: PathBuf, + content: Vec, + mode: FileMode, + oid: String, +} + +/// Tracks sets of changes to files and exposes the overall unified diff. +/// Internally, the way this works is now: +/// 1. Maintain an in-memory baseline snapshot of files when they are first seen. +/// For new additions, do not create a baseline so that diffs are shown as proper additions (using /dev/null). +/// 2. Keep a stable internal filename (uuid) per external path for rename tracking. +/// 3. To compute the aggregated unified diff, compare each baseline snapshot to the current file on disk entirely in-memory +/// using the `similar` crate and emit unified diffs with rewritten external paths. +#[derive(Default)] +pub struct TurnDiffTracker { + /// Map external path -> internal filename (uuid). + external_to_temp_name: HashMap, + /// Internal filename -> baseline file info. + baseline_file_info: HashMap, + /// Internal filename -> external path as of current accumulated state (after applying all changes). + /// This is where renames are tracked. + temp_name_to_current_path: HashMap, + /// Cache of known git worktree roots to avoid repeated filesystem walks. + git_root_cache: Vec, +} + +impl TurnDiffTracker { + pub fn new() -> Self { + Self::default() + } + + /// Front-run apply patch calls to track the starting contents of any modified files. + /// - Creates an in-memory baseline snapshot for files that already exist on disk when first seen. + /// - For additions, we intentionally do not create a baseline snapshot so that diffs are proper additions. + /// - Also updates internal mappings for move/rename events. + pub fn on_patch_begin(&mut self, changes: &HashMap) { + for (path, change) in changes.iter() { + // Ensure a stable internal filename exists for this external path. + if !self.external_to_temp_name.contains_key(path) { + let internal = Uuid::new_v4().to_string(); + self.external_to_temp_name + .insert(path.clone(), internal.clone()); + self.temp_name_to_current_path + .insert(internal.clone(), path.clone()); + + // If the file exists on disk now, snapshot as baseline; else leave missing to represent /dev/null. + let baseline_file_info = if path.exists() { + let mode = file_mode_for_path(path); + let mode_val = mode.unwrap_or(FileMode::Regular); + let content = blob_bytes(path, &mode_val).unwrap_or_default(); + let oid = if mode == Some(FileMode::Symlink) { + format!("{:x}", git_blob_sha1_hex_bytes(&content)) + } else { + self.git_blob_oid_for_path(path) + .unwrap_or_else(|| format!("{:x}", git_blob_sha1_hex_bytes(&content))) + }; + Some(BaselineFileInfo { + path: path.clone(), + content, + mode: mode_val, + oid, + }) + } else { + Some(BaselineFileInfo { + path: path.clone(), + content: vec![], + mode: FileMode::Regular, + oid: ZERO_OID.to_string(), + }) + }; + + if let Some(baseline_file_info) = baseline_file_info { + self.baseline_file_info + .insert(internal.clone(), baseline_file_info); + } + } + + // Track rename/move in current mapping if provided in an Update. + if let FileChange::Update { + move_path: Some(dest), + .. + } = change + { + let uuid_filename = match self.external_to_temp_name.get(path) { + Some(i) => i.clone(), + None => { + // This should be rare, but if we haven't mapped the source, create it with no baseline. + let i = Uuid::new_v4().to_string(); + self.baseline_file_info.insert( + i.clone(), + BaselineFileInfo { + path: path.clone(), + content: vec![], + mode: FileMode::Regular, + oid: ZERO_OID.to_string(), + }, + ); + i + } + }; + // Update current external mapping for temp file name. + self.temp_name_to_current_path + .insert(uuid_filename.clone(), dest.clone()); + // Update forward file_mapping: external current -> internal name. + self.external_to_temp_name.remove(path); + self.external_to_temp_name + .insert(dest.clone(), uuid_filename); + }; + } + } + + fn get_path_for_internal(&self, internal: &str) -> Option { + self.temp_name_to_current_path + .get(internal) + .cloned() + .or_else(|| { + self.baseline_file_info + .get(internal) + .map(|info| info.path.clone()) + }) + } + + /// Find the git worktree root for a file/directory by walking up to the first ancestor containing a `.git` entry. + /// Uses a simple cache of known roots and avoids negative-result caching for simplicity. + fn find_git_root_cached(&mut self, start: &Path) -> Option { + let dir = if start.is_dir() { + start + } else { + start.parent()? + }; + + // Fast path: if any cached root is an ancestor of this path, use it. + if let Some(root) = self + .git_root_cache + .iter() + .find(|r| dir.starts_with(r)) + .cloned() + { + return Some(root); + } + + // Walk up to find a `.git` marker. + let mut cur = dir.to_path_buf(); + loop { + let git_marker = cur.join(".git"); + if git_marker.is_dir() || git_marker.is_file() { + if !self.git_root_cache.iter().any(|r| r == &cur) { + self.git_root_cache.push(cur.clone()); + } + return Some(cur); + } + + // On Windows, avoid walking above the drive or UNC share root. + #[cfg(windows)] + { + if is_windows_drive_or_unc_root(&cur) { + return None; + } + } + + if let Some(parent) = cur.parent() { + cur = parent.to_path_buf(); + } else { + return None; + } + } + } + + /// Return a display string for `path` relative to its git root if found, else absolute. + fn relative_to_git_root_str(&mut self, path: &Path) -> String { + let s = if let Some(root) = self.find_git_root_cached(path) { + if let Ok(rel) = path.strip_prefix(&root) { + rel.display().to_string() + } else { + path.display().to_string() + } + } else { + path.display().to_string() + }; + s.replace('\\', "/") + } + + /// Ask git to compute the blob SHA-1 for the file at `path` within its repository. + /// Returns None if no repository is found or git invocation fails. + fn git_blob_oid_for_path(&mut self, path: &Path) -> Option { + let root = self.find_git_root_cached(path)?; + // Compute a path relative to the repo root for better portability across platforms. + let rel = path.strip_prefix(&root).unwrap_or(path); + let output = Command::new("git") + .arg("-C") + .arg(&root) + .arg("hash-object") + .arg("--") + .arg(rel) + .output() + .ok()?; + if !output.status.success() { + return None; + } + let s = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if s.len() == 40 { Some(s) } else { None } + } + + /// Recompute the aggregated unified diff by comparing all of the in-memory snapshots that were + /// collected before the first time they were touched by apply_patch during this turn with + /// the current repo state. + pub fn get_unified_diff(&mut self) -> Result> { + let mut aggregated = String::new(); + + // Compute diffs per tracked internal file in a stable order by external path. + let mut baseline_file_names: Vec = + self.baseline_file_info.keys().cloned().collect(); + // Sort lexicographically by full repo-relative path to match git behavior. + baseline_file_names.sort_by_key(|internal| { + self.get_path_for_internal(internal) + .map(|p| self.relative_to_git_root_str(&p)) + .unwrap_or_default() + }); + + for internal in baseline_file_names { + aggregated.push_str(self.get_file_diff(&internal).as_str()); + if !aggregated.ends_with('\n') { + aggregated.push('\n'); + } + } + + if aggregated.trim().is_empty() { + Ok(None) + } else { + Ok(Some(aggregated)) + } + } + + fn get_file_diff(&mut self, internal_file_name: &str) -> String { + let mut aggregated = String::new(); + + // Snapshot lightweight fields only. + let (baseline_external_path, baseline_mode, left_oid) = { + if let Some(info) = self.baseline_file_info.get(internal_file_name) { + (info.path.clone(), info.mode, info.oid.clone()) + } else { + (PathBuf::new(), FileMode::Regular, ZERO_OID.to_string()) + } + }; + let current_external_path = match self.get_path_for_internal(internal_file_name) { + Some(p) => p, + None => return aggregated, + }; + + let current_mode = file_mode_for_path(¤t_external_path).unwrap_or(FileMode::Regular); + let right_bytes = blob_bytes(¤t_external_path, ¤t_mode); + + // Compute displays with &mut self before borrowing any baseline content. + let left_display = self.relative_to_git_root_str(&baseline_external_path); + let right_display = self.relative_to_git_root_str(¤t_external_path); + + // Compute right oid before borrowing baseline content. + let right_oid = if let Some(b) = right_bytes.as_ref() { + if current_mode == FileMode::Symlink { + format!("{:x}", git_blob_sha1_hex_bytes(b)) + } else { + self.git_blob_oid_for_path(¤t_external_path) + .unwrap_or_else(|| format!("{:x}", git_blob_sha1_hex_bytes(b))) + } + } else { + ZERO_OID.to_string() + }; + + // Borrow baseline content only after all &mut self uses are done. + let left_present = left_oid.as_str() != ZERO_OID; + let left_bytes: Option<&[u8]> = if left_present { + self.baseline_file_info + .get(internal_file_name) + .map(|i| i.content.as_slice()) + } else { + None + }; + + // Fast path: identical bytes or both missing. + if left_bytes == right_bytes.as_deref() { + return aggregated; + } + + aggregated.push_str(&format!("diff --git a/{left_display} b/{right_display}\n")); + + let is_add = !left_present && right_bytes.is_some(); + let is_delete = left_present && right_bytes.is_none(); + + if is_add { + aggregated.push_str(&format!("new file mode {current_mode}\n")); + } else if is_delete { + aggregated.push_str(&format!("deleted file mode {baseline_mode}\n")); + } else if baseline_mode != current_mode { + aggregated.push_str(&format!("old mode {baseline_mode}\n")); + aggregated.push_str(&format!("new mode {current_mode}\n")); + } + + let left_text = left_bytes.and_then(|b| std::str::from_utf8(b).ok()); + let right_text = right_bytes + .as_deref() + .and_then(|b| std::str::from_utf8(b).ok()); + + let can_text_diff = matches!( + (left_text, right_text, is_add, is_delete), + (Some(_), Some(_), _, _) | (_, Some(_), true, _) | (Some(_), _, _, true) + ); + + if can_text_diff { + let l = left_text.unwrap_or(""); + let r = right_text.unwrap_or(""); + + aggregated.push_str(&format!("index {left_oid}..{right_oid}\n")); + + let old_header = if left_present { + format!("a/{left_display}") + } else { + DEV_NULL.to_string() + }; + let new_header = if right_bytes.is_some() { + format!("b/{right_display}") + } else { + DEV_NULL.to_string() + }; + + let diff = similar::TextDiff::from_lines(l, r); + let unified = diff + .unified_diff() + .context_radius(3) + .header(&old_header, &new_header) + .to_string(); + + aggregated.push_str(&unified); + } else { + aggregated.push_str(&format!("index {left_oid}..{right_oid}\n")); + let old_header = if left_present { + format!("a/{left_display}") + } else { + DEV_NULL.to_string() + }; + let new_header = if right_bytes.is_some() { + format!("b/{right_display}") + } else { + DEV_NULL.to_string() + }; + aggregated.push_str(&format!("--- {old_header}\n")); + aggregated.push_str(&format!("+++ {new_header}\n")); + aggregated.push_str("Binary files differ\n"); + } + aggregated + } +} + +/// Compute the Git SHA-1 blob object ID for the given content (bytes). +fn git_blob_sha1_hex_bytes(data: &[u8]) -> Output { + // Git blob hash is sha1 of: "blob \0" + let header = format!("blob {}\0", data.len()); + use sha1::Digest; + let mut hasher = sha1::Sha1::new(); + hasher.update(header.as_bytes()); + hasher.update(data); + hasher.finalize() +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum FileMode { + Regular, + #[cfg(unix)] + Executable, + Symlink, +} + +impl FileMode { + fn as_str(&self) -> &'static str { + match self { + FileMode::Regular => "100644", + #[cfg(unix)] + FileMode::Executable => "100755", + FileMode::Symlink => "120000", + } + } +} + +impl std::fmt::Display for FileMode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + +#[cfg(unix)] +fn file_mode_for_path(path: &Path) -> Option { + use std::os::unix::fs::PermissionsExt; + let meta = fs::symlink_metadata(path).ok()?; + let ft = meta.file_type(); + if ft.is_symlink() { + return Some(FileMode::Symlink); + } + let mode = meta.permissions().mode(); + let is_exec = (mode & 0o111) != 0; + Some(if is_exec { + FileMode::Executable + } else { + FileMode::Regular + }) +} + +#[cfg(not(unix))] +fn file_mode_for_path(_path: &Path) -> Option { + // Default to non-executable on non-unix. + Some(FileMode::Regular) +} + +fn blob_bytes(path: &Path, mode: &FileMode) -> Option> { + if path.exists() { + let contents = if *mode == FileMode::Symlink { + symlink_blob_bytes(path) + .ok_or_else(|| anyhow!("failed to read symlink target for {}", path.display())) + } else { + fs::read(path) + .with_context(|| format!("failed to read current file for diff {}", path.display())) + }; + contents.ok() + } else { + None + } +} + +#[cfg(unix)] +fn symlink_blob_bytes(path: &Path) -> Option> { + use std::os::unix::ffi::OsStrExt; + let target = std::fs::read_link(path).ok()?; + Some(target.as_os_str().as_bytes().to_vec()) +} + +#[cfg(not(unix))] +fn symlink_blob_bytes(_path: &Path) -> Option> { + None +} + +#[cfg(windows)] +fn is_windows_drive_or_unc_root(p: &std::path::Path) -> bool { + use std::path::Component; + let mut comps = p.components(); + matches!( + (comps.next(), comps.next(), comps.next()), + (Some(Component::Prefix(_)), Some(Component::RootDir), None) + ) +} + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used)] + use super::*; + use pretty_assertions::assert_eq; + use tempfile::tempdir; + + /// Compute the Git SHA-1 blob object ID for the given content (string). + /// This delegates to the bytes version to avoid UTF-8 lossy conversions here. + fn git_blob_sha1_hex(data: &str) -> String { + format!("{:x}", git_blob_sha1_hex_bytes(data.as_bytes())) + } + + fn normalize_diff_for_test(input: &str, root: &Path) -> String { + let root_str = root.display().to_string().replace('\\', "/"); + let replaced = input.replace(&root_str, ""); + // Split into blocks on lines starting with "diff --git ", sort blocks for determinism, and rejoin + let mut blocks: Vec = Vec::new(); + let mut current = String::new(); + for line in replaced.lines() { + if line.starts_with("diff --git ") && !current.is_empty() { + blocks.push(current); + current = String::new(); + } + if !current.is_empty() { + current.push('\n'); + } + current.push_str(line); + } + if !current.is_empty() { + blocks.push(current); + } + blocks.sort(); + let mut out = blocks.join("\n"); + if !out.ends_with('\n') { + out.push('\n'); + } + out + } + + #[test] + fn accumulates_add_and_update() { + let mut acc = TurnDiffTracker::new(); + + let dir = tempdir().unwrap(); + let file = dir.path().join("a.txt"); + + // First patch: add file (baseline should be /dev/null). + let add_changes = HashMap::from([( + file.clone(), + FileChange::Add { + content: "foo\n".to_string(), + }, + )]); + acc.on_patch_begin(&add_changes); + + // Simulate apply: create the file on disk. + fs::write(&file, "foo\n").unwrap(); + let first = acc.get_unified_diff().unwrap().unwrap(); + let first = normalize_diff_for_test(&first, dir.path()); + let expected_first = { + let mode = file_mode_for_path(&file).unwrap_or(FileMode::Regular); + let right_oid = git_blob_sha1_hex("foo\n"); + format!( + r#"diff --git a//a.txt b//a.txt +new file mode {mode} +index {ZERO_OID}..{right_oid} +--- {DEV_NULL} ++++ b//a.txt +@@ -0,0 +1 @@ ++foo +"#, + ) + }; + assert_eq!(first, expected_first); + + // Second patch: update the file on disk. + let update_changes = HashMap::from([( + file.clone(), + FileChange::Update { + unified_diff: "".to_owned(), + move_path: None, + }, + )]); + acc.on_patch_begin(&update_changes); + + // Simulate apply: append a new line. + fs::write(&file, "foo\nbar\n").unwrap(); + let combined = acc.get_unified_diff().unwrap().unwrap(); + let combined = normalize_diff_for_test(&combined, dir.path()); + let expected_combined = { + let mode = file_mode_for_path(&file).unwrap_or(FileMode::Regular); + let right_oid = git_blob_sha1_hex("foo\nbar\n"); + format!( + r#"diff --git a//a.txt b//a.txt +new file mode {mode} +index {ZERO_OID}..{right_oid} +--- {DEV_NULL} ++++ b//a.txt +@@ -0,0 +1,2 @@ ++foo ++bar +"#, + ) + }; + assert_eq!(combined, expected_combined); + } + + #[test] + fn accumulates_delete() { + let dir = tempdir().unwrap(); + let file = dir.path().join("b.txt"); + fs::write(&file, "x\n").unwrap(); + + let mut acc = TurnDiffTracker::new(); + let del_changes = HashMap::from([(file.clone(), FileChange::Delete)]); + acc.on_patch_begin(&del_changes); + + // Simulate apply: delete the file from disk. + let baseline_mode = file_mode_for_path(&file).unwrap_or(FileMode::Regular); + fs::remove_file(&file).unwrap(); + let diff = acc.get_unified_diff().unwrap().unwrap(); + let diff = normalize_diff_for_test(&diff, dir.path()); + let expected = { + let left_oid = git_blob_sha1_hex("x\n"); + format!( + r#"diff --git a//b.txt b//b.txt +deleted file mode {baseline_mode} +index {left_oid}..{ZERO_OID} +--- a//b.txt ++++ {DEV_NULL} +@@ -1 +0,0 @@ +-x +"#, + ) + }; + assert_eq!(diff, expected); + } + + #[test] + fn accumulates_move_and_update() { + let dir = tempdir().unwrap(); + let src = dir.path().join("src.txt"); + let dest = dir.path().join("dst.txt"); + fs::write(&src, "line\n").unwrap(); + + let mut acc = TurnDiffTracker::new(); + let mv_changes = HashMap::from([( + src.clone(), + FileChange::Update { + unified_diff: "".to_owned(), + move_path: Some(dest.clone()), + }, + )]); + acc.on_patch_begin(&mv_changes); + + // Simulate apply: move and update content. + fs::rename(&src, &dest).unwrap(); + fs::write(&dest, "line2\n").unwrap(); + + let out = acc.get_unified_diff().unwrap().unwrap(); + let out = normalize_diff_for_test(&out, dir.path()); + let expected = { + let left_oid = git_blob_sha1_hex("line\n"); + let right_oid = git_blob_sha1_hex("line2\n"); + format!( + r#"diff --git a//src.txt b//dst.txt +index {left_oid}..{right_oid} +--- a//src.txt ++++ b//dst.txt +@@ -1 +1 @@ +-line ++line2 +"# + ) + }; + assert_eq!(out, expected); + } + + #[test] + fn move_without_1change_yields_no_diff() { + let dir = tempdir().unwrap(); + let src = dir.path().join("moved.txt"); + let dest = dir.path().join("renamed.txt"); + fs::write(&src, "same\n").unwrap(); + + let mut acc = TurnDiffTracker::new(); + let mv_changes = HashMap::from([( + src.clone(), + FileChange::Update { + unified_diff: "".to_owned(), + move_path: Some(dest.clone()), + }, + )]); + acc.on_patch_begin(&mv_changes); + + // Simulate apply: move only, no content change. + fs::rename(&src, &dest).unwrap(); + + let diff = acc.get_unified_diff().unwrap(); + assert_eq!(diff, None); + } + + #[test] + fn move_declared_but_file_only_appears_at_dest_is_add() { + let dir = tempdir().unwrap(); + let src = dir.path().join("src.txt"); + let dest = dir.path().join("dest.txt"); + let mut acc = TurnDiffTracker::new(); + let mv = HashMap::from([( + src.clone(), + FileChange::Update { + unified_diff: "".into(), + move_path: Some(dest.clone()), + }, + )]); + acc.on_patch_begin(&mv); + // No file existed initially; create only dest + fs::write(&dest, "hello\n").unwrap(); + let diff = acc.get_unified_diff().unwrap().unwrap(); + let diff = normalize_diff_for_test(&diff, dir.path()); + let expected = { + let mode = file_mode_for_path(&dest).unwrap_or(FileMode::Regular); + let right_oid = git_blob_sha1_hex("hello\n"); + format!( + r#"diff --git a//src.txt b//dest.txt +new file mode {mode} +index {ZERO_OID}..{right_oid} +--- {DEV_NULL} ++++ b//dest.txt +@@ -0,0 +1 @@ ++hello +"#, + ) + }; + assert_eq!(diff, expected); + } + + #[test] + fn update_persists_across_new_baseline_for_new_file() { + let dir = tempdir().unwrap(); + let a = dir.path().join("a.txt"); + let b = dir.path().join("b.txt"); + fs::write(&a, "foo\n").unwrap(); + fs::write(&b, "z\n").unwrap(); + + let mut acc = TurnDiffTracker::new(); + + // First: update existing a.txt (baseline snapshot is created for a). + let update_a = HashMap::from([( + a.clone(), + FileChange::Update { + unified_diff: "".to_owned(), + move_path: None, + }, + )]); + acc.on_patch_begin(&update_a); + // Simulate apply: modify a.txt on disk. + fs::write(&a, "foo\nbar\n").unwrap(); + let first = acc.get_unified_diff().unwrap().unwrap(); + let first = normalize_diff_for_test(&first, dir.path()); + let expected_first = { + let left_oid = git_blob_sha1_hex("foo\n"); + let right_oid = git_blob_sha1_hex("foo\nbar\n"); + format!( + r#"diff --git a//a.txt b//a.txt +index {left_oid}..{right_oid} +--- a//a.txt ++++ b//a.txt +@@ -1 +1,2 @@ + foo ++bar +"# + ) + }; + assert_eq!(first, expected_first); + + // Next: introduce a brand-new path b.txt into baseline snapshots via a delete change. + let del_b = HashMap::from([(b.clone(), FileChange::Delete)]); + acc.on_patch_begin(&del_b); + // Simulate apply: delete b.txt. + let baseline_mode = file_mode_for_path(&b).unwrap_or(FileMode::Regular); + fs::remove_file(&b).unwrap(); + + let combined = acc.get_unified_diff().unwrap().unwrap(); + let combined = normalize_diff_for_test(&combined, dir.path()); + let expected = { + let left_oid_a = git_blob_sha1_hex("foo\n"); + let right_oid_a = git_blob_sha1_hex("foo\nbar\n"); + let left_oid_b = git_blob_sha1_hex("z\n"); + format!( + r#"diff --git a//a.txt b//a.txt +index {left_oid_a}..{right_oid_a} +--- a//a.txt ++++ b//a.txt +@@ -1 +1,2 @@ + foo ++bar +diff --git a//b.txt b//b.txt +deleted file mode {baseline_mode} +index {left_oid_b}..{ZERO_OID} +--- a//b.txt ++++ {DEV_NULL} +@@ -1 +0,0 @@ +-z +"#, + ) + }; + assert_eq!(combined, expected); + } + + #[test] + fn binary_files_differ_update() { + let dir = tempdir().unwrap(); + let file = dir.path().join("bin.dat"); + + // Initial non-UTF8 bytes + let left_bytes: Vec = vec![0xff, 0xfe, 0xfd, 0x00]; + // Updated non-UTF8 bytes + let right_bytes: Vec = vec![0x01, 0x02, 0x03, 0x00]; + + fs::write(&file, &left_bytes).unwrap(); + + let mut acc = TurnDiffTracker::new(); + let update_changes = HashMap::from([( + file.clone(), + FileChange::Update { + unified_diff: "".to_owned(), + move_path: None, + }, + )]); + acc.on_patch_begin(&update_changes); + + // Apply update on disk + fs::write(&file, &right_bytes).unwrap(); + + let diff = acc.get_unified_diff().unwrap().unwrap(); + let diff = normalize_diff_for_test(&diff, dir.path()); + let expected = { + let left_oid = format!("{:x}", git_blob_sha1_hex_bytes(&left_bytes)); + let right_oid = format!("{:x}", git_blob_sha1_hex_bytes(&right_bytes)); + format!( + r#"diff --git a//bin.dat b//bin.dat +index {left_oid}..{right_oid} +--- a//bin.dat ++++ b//bin.dat +Binary files differ +"# + ) + }; + assert_eq!(diff, expected); + } + + #[test] + fn filenames_with_spaces_add_and_update() { + let mut acc = TurnDiffTracker::new(); + + let dir = tempdir().unwrap(); + let file = dir.path().join("name with spaces.txt"); + + // First patch: add file (baseline should be /dev/null). + let add_changes = HashMap::from([( + file.clone(), + FileChange::Add { + content: "foo\n".to_string(), + }, + )]); + acc.on_patch_begin(&add_changes); + + // Simulate apply: create the file on disk. + fs::write(&file, "foo\n").unwrap(); + let first = acc.get_unified_diff().unwrap().unwrap(); + let first = normalize_diff_for_test(&first, dir.path()); + let expected_first = { + let mode = file_mode_for_path(&file).unwrap_or(FileMode::Regular); + let right_oid = git_blob_sha1_hex("foo\n"); + format!( + r#"diff --git a//name with spaces.txt b//name with spaces.txt +new file mode {mode} +index {ZERO_OID}..{right_oid} +--- {DEV_NULL} ++++ b//name with spaces.txt +@@ -0,0 +1 @@ ++foo +"#, + ) + }; + assert_eq!(first, expected_first); + + // Second patch: update the file on disk. + let update_changes = HashMap::from([( + file.clone(), + FileChange::Update { + unified_diff: "".to_owned(), + move_path: None, + }, + )]); + acc.on_patch_begin(&update_changes); + + // Simulate apply: append a new line with a space. + fs::write(&file, "foo\nbar baz\n").unwrap(); + let combined = acc.get_unified_diff().unwrap().unwrap(); + let combined = normalize_diff_for_test(&combined, dir.path()); + let expected_combined = { + let mode = file_mode_for_path(&file).unwrap_or(FileMode::Regular); + let right_oid = git_blob_sha1_hex("foo\nbar baz\n"); + format!( + r#"diff --git a//name with spaces.txt b//name with spaces.txt +new file mode {mode} +index {ZERO_OID}..{right_oid} +--- {DEV_NULL} ++++ b//name with spaces.txt +@@ -0,0 +1,2 @@ ++foo ++bar baz +"#, + ) + }; + assert_eq!(combined, expected_combined); + } +} 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 72e2f9298f..c290d9336b 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -20,6 +20,7 @@ use codex_core::protocol::PatchApplyEndEvent; use codex_core::protocol::SessionConfiguredEvent; use codex_core::protocol::TaskCompleteEvent; use codex_core::protocol::TokenUsage; +use codex_core::protocol::TurnDiffEvent; use owo_colors::OwoColorize; use owo_colors::Style; use shlex::try_join; @@ -399,6 +400,7 @@ impl EventProcessor for EventProcessorWithHumanOutput { stdout, stderr, success, + .. }) => { let patch_begin = self.call_id_to_patch.remove(&call_id); @@ -428,6 +430,10 @@ impl EventProcessor for EventProcessorWithHumanOutput { println!("{}", line.style(self.dimmed)); } } + EventMsg::TurnDiff(TurnDiffEvent { unified_diff }) => { + ts_println!(self, "{}", "turn diff:".style(self.magenta)); + println!("{unified_diff}"); + } EventMsg::ExecApprovalRequest(_) => { // Should we exit? } diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index d489ffe076..205dfa4631 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -263,6 +263,7 @@ async fn run_codex_tool_session_inner( | EventMsg::BackgroundEvent(_) | EventMsg::PatchApplyBegin(_) | EventMsg::PatchApplyEnd(_) + | EventMsg::TurnDiff(_) | EventMsg::GetHistoryEntryResponse(_) | EventMsg::PlanUpdate(_) | EventMsg::ShutdownComplete => { diff --git a/codex-rs/mcp-server/src/conversation_loop.rs b/codex-rs/mcp-server/src/conversation_loop.rs index 534275181a..1db39a2306 100644 --- a/codex-rs/mcp-server/src/conversation_loop.rs +++ b/codex-rs/mcp-server/src/conversation_loop.rs @@ -97,6 +97,7 @@ pub async fn run_conversation_loop( | EventMsg::McpToolCallEnd(_) | EventMsg::ExecCommandBegin(_) | EventMsg::ExecCommandEnd(_) + | EventMsg::TurnDiff(_) | EventMsg::BackgroundEvent(_) | EventMsg::ExecCommandOutputDelta(_) | EventMsg::PatchApplyBegin(_) From dc15a5cf0b50eaa7a75dca754d618090d41d51e3 Mon Sep 17 00:00:00 2001 From: ae Date: Mon, 4 Aug 2025 09:34:46 -0700 Subject: [PATCH 02/12] feat: accept custom instructions in profiles (#1803) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allows users to set their experimental_instructions_file in configs. For example the below enables experimental instructions when running `codex -p foo`. ``` [profiles.foo] experimental_instructions_file = "/Users/foo/.codex/prompt.md" ``` # Testing - ✅ Running against a profile with experimental_instructions_file works. - ✅ Running against a profile without experimental_instructions_file works. - ✅ Running against no profile with experimental_instructions_file works. - ✅ Running against no profile without experimental_instructions_file works. --- codex-rs/core/src/config.rs | 10 ++++++---- codex-rs/core/src/config_profile.rs | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 87629d821a..b43dc56ba0 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -480,10 +480,12 @@ impl Config { // Load base instructions override from a file if specified. If the // path is relative, resolve it against the effective cwd so the // behaviour matches other path-like config values. - let file_base_instructions = Self::get_base_instructions( - cfg.experimental_instructions_file.as_ref(), - &resolved_cwd, - )?; + let experimental_instructions_path = config_profile + .experimental_instructions_file + .as_ref() + .or(cfg.experimental_instructions_file.as_ref()); + let file_base_instructions = + Self::get_base_instructions(experimental_instructions_path, &resolved_cwd)?; let base_instructions = base_instructions.or(file_base_instructions); let config = Self { diff --git a/codex-rs/core/src/config_profile.rs b/codex-rs/core/src/config_profile.rs index 176a9b1500..d945d1df7a 100644 --- a/codex-rs/core/src/config_profile.rs +++ b/codex-rs/core/src/config_profile.rs @@ -1,4 +1,5 @@ use serde::Deserialize; +use std::path::PathBuf; use crate::config_types::ReasoningEffort; use crate::config_types::ReasoningSummary; @@ -17,4 +18,5 @@ pub struct ConfigProfile { pub model_reasoning_effort: Option, pub model_reasoning_summary: Option, pub chatgpt_base_url: Option, + pub experimental_instructions_file: Option, } From a6139aa0035d19d794a3669d6196f9f32a8c8352 Mon Sep 17 00:00:00 2001 From: easong-openai Date: Mon, 4 Aug 2025 10:42:39 -0700 Subject: [PATCH 03/12] Update prompt.md (#1819) The existing prompt is really bad. As a low-hanging fruit, let's correct the apply_patch instructions - this helps smaller models successfully apply patches. --- codex-rs/core/prompt.md | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/codex-rs/core/prompt.md b/codex-rs/core/prompt.md index 0a4578270a..4e55003b9f 100644 --- a/codex-rs/core/prompt.md +++ b/codex-rs/core/prompt.md @@ -10,7 +10,7 @@ You MUST adhere to the following criteria when executing the task: - Showing user code and tool call details is allowed. - User instructions may overwrite the _CODING GUIDELINES_ section in this developer message. - Do not use \`ls -R\`, \`find\`, or \`grep\` - these are slow in large repos. Use \`rg\` and \`rg --files\`. -- Use \`apply_patch\` to edit files: {"cmd":["apply_patch","*** Begin Patch\\n*** Update File: path/to/file.py\\n@@ def example():\\n- pass\\n+ return 123\\n*** End Patch"]} +- Use \`apply_patch\` to edit files: {"command":["apply_patch","*** Begin Patch\\n*** Update File: path/to/file.py\\n@@ def example():\\n- pass\\n+ return 123\\n*** End Patch"]} - If completing the user's task requires writing or modifying files: - Your code and final answer should follow these _CODING GUIDELINES_: - Fix the problem at the root cause rather than applying surface-level patches, when possible. @@ -40,16 +40,16 @@ You MUST adhere to the following criteria when executing the task: Your patch language is a stripped‑down, file‑oriented diff format designed to be easy to parse and safe to apply. You can think of it as a high‑level envelope: -**_ Begin Patch +*** Begin Patch [ one or more file sections ] -_** End Patch +*** End Patch Within that envelope, you get a sequence of file operations. You MUST include a header to specify the action you are taking. Each operation starts with one of three headers: -**_ Add File: - create a new file. Every following line is a + line (the initial contents). -_** Delete File: - remove an existing file. Nothing follows. +*** Add File: - create a new file. Every following line is a + line (the initial contents). +*** Delete File: - remove an existing file. Nothing follows. \*\*\* Update File: - patch an existing file in place (optionally with a rename). May be immediately followed by \*\*\* Move to: if you want to rename the file. @@ -63,28 +63,28 @@ Within a hunk each line starts with: At the end of a truncated hunk you can emit \*\*\* End of File. Patch := Begin { FileOp } End -Begin := "**_ Begin Patch" NEWLINE -End := "_** End Patch" NEWLINE +Begin := "*** Begin Patch" NEWLINE +End := "*** End Patch" NEWLINE FileOp := AddFile | DeleteFile | UpdateFile -AddFile := "**_ Add File: " path NEWLINE { "+" line NEWLINE } -DeleteFile := "_** Delete File: " path NEWLINE -UpdateFile := "**_ Update File: " path NEWLINE [ MoveTo ] { Hunk } -MoveTo := "_** Move to: " newPath NEWLINE +AddFile := "*** Add File: " path NEWLINE { "+" line NEWLINE } +DeleteFile := "*** Delete File: " path NEWLINE +UpdateFile := "*** Update File: " path NEWLINE [ MoveTo ] { Hunk } +MoveTo := "*** Move to: " newPath NEWLINE Hunk := "@@" [ header ] NEWLINE { HunkLine } [ "*** End of File" NEWLINE ] HunkLine := (" " | "-" | "+") text NEWLINE A full patch can combine several operations: -**_ Begin Patch -_** Add File: hello.txt +*** Begin Patch +*** Add File: hello.txt +Hello world -**_ Update File: src/app.py -_** Move to: src/main.py +*** Update File: src/app.py +*** Move to: src/main.py @@ def greet(): -print("Hi") +print("Hello, world!") -**_ Delete File: obsolete.txt -_** End Patch +*** Delete File: obsolete.txt +*** End Patch It is important to remember: @@ -101,7 +101,7 @@ Plan updates A tool named `update_plan` is available. Use it to keep an up‑to‑date, step‑by‑step plan for the task so you can follow your progress. When making your plans, keep in mind that you are a deployed coding agent - `update_plan` calls should not involve doing anything that you aren't capable of doing. For example, `update_plan` calls should NEVER contain tasks to merge your own pull requests. Only stop to ask the user if you genuinely need their feedback on a change. -- At the start of the task, call `update_plan` with an initial plan: a short list of 1‑sentence steps with a `status` for each step (`pending`, `in_progress`, or `completed`). There should always be exactly one `in_progress` step until everything is done. +- At the start of any nontrivial task, call `update_plan` with an initial plan: a short list of 1‑sentence steps with a `status` for each step (`pending`, `in_progress`, or `completed`). There should always be exactly one `in_progress` step until everything is done. - Whenever you finish a step, call `update_plan` again, marking the finished step as `completed` and the next step as `in_progress`. - If your plan needs to change, call `update_plan` with the revised steps and include an `explanation` describing the change. - When all steps are complete, make a final `update_plan` call with all steps marked `completed`. From 64cfbbd3c8609fb1c1a1bfbc1bf86148e12e6cc4 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Mon, 4 Aug 2025 11:25:01 -0700 Subject: [PATCH 04/12] support more keys in textarea (#1820) Added: * C-m for newline (not sure if this is actually treated differently to Enter, but tui-textarea handles it and it doesn't hurt) * C-d to delete one char forwards (same as Del) * A-bksp to delete backwards one word * A-arrows to navigate by word --- codex-rs/tui/src/bottom_pane/textarea.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index e150135b75..cb30c2ac7a 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -210,7 +210,7 @@ impl TextArea { .. } => self.insert_str(&c.to_string()), KeyEvent { - code: KeyCode::Char('j'), + code: KeyCode::Char('j' | 'm'), modifiers: KeyModifiers::CONTROL, .. } @@ -220,11 +220,22 @@ impl TextArea { } => self.insert_str("\n"), KeyEvent { code: KeyCode::Backspace, + modifiers: KeyModifiers::ALT, + .. + } => self.delete_backward_word(), + KeyEvent { + code: KeyCode::Backspace, + modifiers: KeyModifiers::NONE, .. } => self.delete_backward(1), KeyEvent { code: KeyCode::Delete, .. + } + | KeyEvent { + code: KeyCode::Char('d'), + modifiers: KeyModifiers::CONTROL, + .. } => self.delete_forward(1), KeyEvent { @@ -303,14 +314,14 @@ impl TextArea { } KeyEvent { code: KeyCode::Left, - modifiers: KeyModifiers::CONTROL, + modifiers: KeyModifiers::CONTROL | KeyModifiers::ALT, .. } => { self.set_cursor(self.beginning_of_previous_word()); } KeyEvent { code: KeyCode::Right, - modifiers: KeyModifiers::CONTROL, + modifiers: KeyModifiers::CONTROL | KeyModifiers::ALT, .. } => { self.set_cursor(self.end_of_next_word()); From 2899817c94098caf96009c1d797597df1c298e3f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 4 Aug 2025 14:24:19 -0700 Subject: [PATCH 05/12] chore(deps): bump toml from 0.9.2 to 0.9.4 in /codex-rs (#1815) Bumps [toml](https://github.com/toml-rs/toml) from 0.9.2 to 0.9.4.
Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=toml&package-manager=cargo&previous-version=0.9.2&new-version=0.9.4)](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 | 10 +++++----- codex-rs/core/Cargo.toml | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index eb4eccd897..9d8a027c53 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -661,7 +661,7 @@ dependencies = [ "clap", "codex-core", "serde", - "toml 0.9.2", + "toml 0.9.4", ] [[package]] @@ -707,7 +707,7 @@ dependencies = [ "tokio", "tokio-test", "tokio-util", - "toml 0.9.2", + "toml 0.9.4", "tracing", "tree-sitter", "tree-sitter-bash", @@ -831,7 +831,7 @@ dependencies = [ "tempfile", "tokio", "tokio-test", - "toml 0.9.2", + "toml 0.9.4", "tracing", "tracing-subscriber", "uuid", @@ -4773,9 +4773,9 @@ dependencies = [ [[package]] name = "toml" -version = "0.9.2" +version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed0aee96c12fa71097902e0bb061a5e1ebd766a6636bb605ba401c45c1650eac" +checksum = "41ae868b5a0f67631c14589f7e250c1ea2c574ee5ba21c6c8dd4b1485705a5a1" dependencies = [ "indexmap 2.10.0", "serde", diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 466e9adf02..e9d6970ded 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -46,7 +46,7 @@ tokio = { version = "1", features = [ "signal", ] } tokio-util = "0.7.14" -toml = "0.9.2" +toml = "0.9.4" tracing = { version = "0.1.41", features = ["log"] } tree-sitter = "0.25.8" tree-sitter-bash = "0.25.0" From 6db597ec0c6833018973bbcbe97139d100913304 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 4 Aug 2025 14:25:00 -0700 Subject: [PATCH 06/12] chore(deps-dev): bump typescript from 5.8.3 to 5.9.2 in /.github/actions/codex (#1814) [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=typescript&package-manager=bun&previous-version=5.8.3&new-version=5.9.2)](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> --- .github/actions/codex/bun.lock | 4 ++-- .github/actions/codex/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/actions/codex/bun.lock b/.github/actions/codex/bun.lock index 8b546a5ac6..82e12cc4b6 100644 --- a/.github/actions/codex/bun.lock +++ b/.github/actions/codex/bun.lock @@ -11,7 +11,7 @@ "@types/bun": "^1.2.19", "@types/node": "^24.1.0", "prettier": "^3.6.2", - "typescript": "^5.8.3", + "typescript": "^5.9.2", }, }, }, @@ -68,7 +68,7 @@ "tunnel": ["tunnel@0.0.6", "", {}, "sha512-1h/Lnq9yajKY2PEbBadPXj3VxsDDu844OnaAo52UVmIzIvwwtBPIuNvkjuzBlTWpfJyUbG3ez0KSBibQkj4ojg=="], - "typescript": ["typescript@5.8.3", "", { "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" } }, "sha512-p1diW6TqL9L07nNxvRMM7hMMw4c5XOo/1ibL4aAIGmSAt9slTE1Xgw5KWuof2uTOvCg9BY7ZRi+GaF+7sfgPeQ=="], + "typescript": ["typescript@5.9.2", "", { "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" } }, "sha512-CWBzXQrc/qOkhidw1OzBTQuYRbfyxDXJMVJ1XNwUHGROVmuaeiEm3OslpZ1RV96d7SKKjZKrSJu3+t/xlw3R9A=="], "undici": ["undici@5.29.0", "", { "dependencies": { "@fastify/busboy": "^2.0.0" } }, "sha512-raqeBD6NQK4SkWhQzeYKd1KmIG6dllBOTt55Rmkt4HtI9mwdWtJljnrXjAFUBLTSN67HWrOIZ3EPF4kjUw80Bg=="], diff --git a/.github/actions/codex/package.json b/.github/actions/codex/package.json index 21817b8a59..6c7ae9002b 100644 --- a/.github/actions/codex/package.json +++ b/.github/actions/codex/package.json @@ -16,6 +16,6 @@ "@types/bun": "^1.2.19", "@types/node": "^24.1.0", "prettier": "^3.6.2", - "typescript": "^5.8.3" + "typescript": "^5.9.2" } } From 89ab5c3f74f6efcf5ac3b5ddb7390a90aecc9df3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 4 Aug 2025 14:26:14 -0700 Subject: [PATCH 07/12] chore(deps): bump serde_json from 1.0.141 to 1.0.142 in /codex-rs (#1817) 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.141 to 1.0.142.
Release notes

Sourced from serde_json's releases.

v1.0.142

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=serde_json&package-manager=cargo&previous-version=1.0.141&new-version=1.0.142)](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 9d8a027c53..0ad32c3cd4 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3997,9 +3997,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.141" +version = "1.0.142" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30b9eff21ebe718216c6ec64e1d9ac57087aad11efc64e32002bce4a0d4c03d3" +checksum = "030fedb782600dcbd6f02d479bf0d817ac3bb40d644745b769d6a96bc3afc5a7" dependencies = [ "indexmap 2.10.0", "itoa", diff --git a/codex-rs/execpolicy/Cargo.toml b/codex-rs/execpolicy/Cargo.toml index ad003e66c4..9693d5c41f 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.110" +serde_json = "1.0.142" 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 3f70377183..bf1e8e687f 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.110" +serde_json = "1.0.142" tokio = { version = "1", features = ["full"] } From 7279080edd35fb005d3c02200d1b06165b57f7dc Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 4 Aug 2025 14:50:53 -0700 Subject: [PATCH 08/12] chore(deps): bump tokio from 1.46.1 to 1.47.1 in /codex-rs (#1816) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [tokio](https://github.com/tokio-rs/tokio) from 1.46.1 to 1.47.1.
Release notes

Sourced from tokio's releases.

Tokio v1.47.1

1.47.1 (August 1st, 2025)

Fixed

  • process: fix panic from spurious pidfd wakeup (#7494)
  • sync: fix broken link of Python asyncio.Event in SetOnce docs (#7485)

#7485: tokio-rs/tokio#7485 #7494: tokio-rs/tokio#7494

Tokio v1.47.0

1.47.0 (July 25th, 2025)

This release adds poll_proceed and cooperative to the coop module for cooperative scheduling, adds SetOnce to the sync module which provides similar functionality to [std::sync::OnceLock], and adds a new method sync::Notify::notified_owned() which returns an OwnedNotified without a lifetime parameter.

Added

  • coop: add cooperative and poll_proceed (#7405)
  • sync: add SetOnce (#7418)
  • sync: add sync::Notify::notified_owned() (#7465)

Changed

  • deps: upgrade windows-sys 0.52 → 0.59 (#7117)
  • deps: update to socket2 v0.6 (#7443)
  • sync: improve AtomicWaker::wake performance (#7450)

Documented

  • metrics: fix listed feature requirements for some metrics (#7449)
  • runtime: improve safety comments of Readiness<'_> (#7415)

#7405: tokio-rs/tokio#7405 #7415: tokio-rs/tokio#7415 #7418: tokio-rs/tokio#7418 #7449: tokio-rs/tokio#7449 #7450: tokio-rs/tokio#7450 #7465: tokio-rs/tokio#7465

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=tokio&package-manager=cargo&previous-version=1.46.1&new-version=1.47.1)](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 | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 0ad32c3cd4..4daae977b0 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2017,7 +2017,7 @@ dependencies = [ "libc", "percent-encoding", "pin-project-lite", - "socket2 0.6.0", + "socket2", "system-configuration", "tokio", "tower-service", @@ -4188,16 +4188,6 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" -[[package]] -name = "socket2" -version = "0.5.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e22376abed350d73dd1cd119b57ffccad95b4e585a7cda43e286245ce23c0678" -dependencies = [ - "libc", - "windows-sys 0.52.0", -] - [[package]] name = "socket2" version = "0.6.0" @@ -4673,9 +4663,9 @@ dependencies = [ [[package]] name = "tokio" -version = "1.46.1" +version = "1.47.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0cc3a2344dafbe23a245241fe8b09735b521110d30fcefbbd5feb1797ca35d17" +checksum = "89e49afdadebb872d3145a5638b59eb0691ea23e46ca484037cfab3b76b95038" dependencies = [ "backtrace", "bytes", @@ -4686,9 +4676,9 @@ dependencies = [ "pin-project-lite", "signal-hook-registry", "slab", - "socket2 0.5.10", + "socket2", "tokio-macros", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] From 3f13ebce10209ab3645f51e7606892b3fd71d47e Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 4 Aug 2025 15:56:32 -0700 Subject: [PATCH 09/12] [codex] stop printing error message when --output-last-message is not specified (#1828) Previously, `codex exec` was printing `Warning: no file to write last message to` as a warning to stderr even though `--output-last-message` was not specified, which is wrong. This fixes the code and changes `handle_last_message()` so that it is only called when `last_message_path` is `Some`. --- codex-rs/exec/src/event_processor.rs | 22 +++++++------------ .../src/event_processor_with_human_output.rs | 7 +++--- .../src/event_processor_with_json_output.rs | 7 +++--- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/codex-rs/exec/src/event_processor.rs b/codex-rs/exec/src/event_processor.rs index 741f89d7cb..0f189f3fa2 100644 --- a/codex-rs/exec/src/event_processor.rs +++ b/codex-rs/exec/src/event_processor.rs @@ -44,20 +44,14 @@ pub(crate) fn create_config_summary_entries(config: &Config) -> Vec<(&'static st entries } -pub(crate) fn handle_last_message( - last_agent_message: Option<&str>, - last_message_path: Option<&Path>, -) { - match (last_message_path, last_agent_message) { - (Some(path), Some(msg)) => write_last_message_file(msg, Some(path)), - (Some(path), None) => { - write_last_message_file("", Some(path)); - eprintln!( - "Warning: no last agent message; wrote empty content to {}", - path.display() - ); - } - (None, _) => eprintln!("Warning: no file to write last message to."), +pub(crate) fn handle_last_message(last_agent_message: Option<&str>, output_file: &Path) { + let message = last_agent_message.unwrap_or_default(); + write_last_message_file(message, Some(output_file)); + if last_agent_message.is_none() { + eprintln!( + "Warning: no last agent message; wrote empty content to {}", + output_file.display() + ); } } 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 c290d9336b..7703c138fc 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -170,10 +170,9 @@ impl EventProcessor for EventProcessorWithHumanOutput { // Ignore. } EventMsg::TaskComplete(TaskCompleteEvent { last_agent_message }) => { - handle_last_message( - last_agent_message.as_deref(), - self.last_message_path.as_deref(), - ); + if let Some(output_file) = self.last_message_path.as_deref() { + handle_last_message(last_agent_message.as_deref(), output_file); + } return CodexStatus::InitiateShutdown; } EventMsg::TokenCount(TokenUsage { total_tokens, .. }) => { diff --git a/codex-rs/exec/src/event_processor_with_json_output.rs b/codex-rs/exec/src/event_processor_with_json_output.rs index e7a658b76f..1d153add6e 100644 --- a/codex-rs/exec/src/event_processor_with_json_output.rs +++ b/codex-rs/exec/src/event_processor_with_json_output.rs @@ -46,10 +46,9 @@ impl EventProcessor for EventProcessorWithJsonOutput { CodexStatus::Running } EventMsg::TaskComplete(TaskCompleteEvent { last_agent_message }) => { - handle_last_message( - last_agent_message.as_deref(), - self.last_message_path.as_deref(), - ); + if let Some(output_file) = self.last_message_path.as_deref() { + handle_last_message(last_agent_message.as_deref(), output_file); + } CodexStatus::InitiateShutdown } EventMsg::ShutdownComplete => CodexStatus::Shutdown, From bd171e5206465593a616cd65344a77d17477d51f Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 4 Aug 2025 16:49:42 -0700 Subject: [PATCH 10/12] add raw reasoning --- codex-rs/core/src/chat_completions.rs | 163 ++++++++++++++---- codex-rs/core/src/client.rs | 10 +- codex-rs/core/src/codex.rs | 60 +++++-- codex-rs/core/src/config.rs | 27 ++- codex-rs/core/src/models.rs | 8 + codex-rs/core/src/protocol.rs | 8 + .../src/event_processor_with_human_output.rs | 9 + codex-rs/mcp-server/src/codex_tool_runner.rs | 3 +- codex-rs/mcp-server/src/conversation_loop.rs | 3 +- 9 files changed, 236 insertions(+), 55 deletions(-) diff --git a/codex-rs/core/src/chat_completions.rs b/codex-rs/core/src/chat_completions.rs index 5ede774b1c..d1b8338987 100644 --- a/codex-rs/core/src/chat_completions.rs +++ b/codex-rs/core/src/chat_completions.rs @@ -207,6 +207,7 @@ async fn process_chat_sse( } let mut fn_call_state = FunctionCallState::default(); + let mut assistant_text = String::new(); loop { let sse = match timeout(idle_timeout, stream.next()).await { @@ -254,21 +255,42 @@ async fn process_chat_sse( let choice_opt = chunk.get("choices").and_then(|c| c.get(0)); if let Some(choice) = choice_opt { - // Handle assistant content tokens. + // Handle assistant content tokens as streaming deltas. if let Some(content) = choice .get("delta") .and_then(|d| d.get("content")) .and_then(|c| c.as_str()) { - let item = ResponseItem::Message { - role: "assistant".to_string(), - content: vec![ContentItem::OutputText { - text: content.to_string(), - }], - id: None, - }; + if !content.is_empty() { + assistant_text.push_str(content); + let _ = tx_event + .send(Ok(ResponseEvent::OutputTextDelta(content.to_string()))) + .await; + } + } - let _ = tx_event.send(Ok(ResponseEvent::OutputItemDone(item))).await; + // Forward any reasoning/thinking deltas if present. + if let Some(reasoning) = choice + .get("delta") + .and_then(|d| d.get("reasoning")) + .and_then(|c| c.as_str()) + { + let _ = tx_event + .send(Ok(ResponseEvent::ReasoningSummaryDelta( + reasoning.to_string(), + ))) + .await; + } + if let Some(reasoning_content) = choice + .get("delta") + .and_then(|d| d.get("reasoning_content")) + .and_then(|c| c.as_str()) + { + let _ = tx_event + .send(Ok(ResponseEvent::ReasoningSummaryDelta( + reasoning_content.to_string(), + ))) + .await; } // Handle streaming function / tool calls. @@ -317,7 +339,18 @@ async fn process_chat_sse( let _ = tx_event.send(Ok(ResponseEvent::OutputItemDone(item))).await; } "stop" => { - // Regular turn without tool-call. + // Regular turn without tool-call. Emit the final assistant message + // as a single OutputItemDone so non-delta consumers see the result. + if !assistant_text.is_empty() { + let item = ResponseItem::Message { + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { + text: std::mem::take(&mut assistant_text), + }], + id: None, + }; + let _ = tx_event.send(Ok(ResponseEvent::OutputItemDone(item))).await; + } } _ => {} } @@ -358,7 +391,10 @@ async fn process_chat_sse( pub(crate) struct AggregatedChatStream { inner: S, cumulative: String, - pending_completed: Option, + cumulative_reasoning: String, + pending: std::collections::VecDeque, + // When true, do not emit a cumulative assistant message at Completed. + streaming_mode: bool, } impl Stream for AggregatedChatStream @@ -370,8 +406,8 @@ where fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { let this = self.get_mut(); - // First, flush any buffered Completed event from the previous call. - if let Some(ev) = this.pending_completed.take() { + // First, flush any buffered events from the previous call. + if let Some(ev) = this.pending.pop_front() { return Poll::Ready(Some(Ok(ev))); } @@ -388,16 +424,21 @@ where let is_assistant_delta = matches!(&item, crate::models::ResponseItem::Message { role, .. } if role == "assistant"); if is_assistant_delta { - if let crate::models::ResponseItem::Message { content, .. } = &item { - if let Some(text) = content.iter().find_map(|c| match c { - crate::models::ContentItem::OutputText { text } => Some(text), - _ => None, - }) { - this.cumulative.push_str(text); + // 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() { + if let crate::models::ResponseItem::Message { content, .. } = &item { + if let Some(text) = content.iter().find_map(|c| match c { + crate::models::ContentItem::OutputText { text } => Some(text), + _ => None, + }) { + this.cumulative.push_str(text); + } } } - // Swallow partial assistant chunk; keep polling. + // Swallow assistant message here; emit on Completed. continue; } @@ -408,24 +449,48 @@ where response_id, token_usage, }))) => { + // Build any aggregated items in the correct order: Reasoning first, then Message. + let mut emitted_any = false; + + if !this.cumulative_reasoning.is_empty() { + let aggregated_reasoning = crate::models::ResponseItem::Reasoning { + id: String::new(), + summary: vec![ + crate::models::ReasoningItemReasoningSummary::SummaryText { + text: std::mem::take(&mut this.cumulative_reasoning), + }, + ], + content: None, + encrypted_content: None, + }; + this.pending + .push_back(ResponseEvent::OutputItemDone(aggregated_reasoning)); + emitted_any = true; + } + if !this.cumulative.is_empty() { - let aggregated_item = crate::models::ResponseItem::Message { + let aggregated_message = crate::models::ResponseItem::Message { id: None, role: "assistant".to_string(), content: vec![crate::models::ContentItem::OutputText { text: std::mem::take(&mut this.cumulative), }], }; + this.pending + .push_back(ResponseEvent::OutputItemDone(aggregated_message)); + emitted_any = true; + } - // Buffer Completed so it is returned *after* the aggregated message. - this.pending_completed = Some(ResponseEvent::Completed { - response_id, - token_usage, + // Always emit Completed last when anything was aggregated. + if emitted_any { + this.pending.push_back(ResponseEvent::Completed { + response_id: response_id.clone(), + token_usage: token_usage.clone(), }); - - return Poll::Ready(Some(Ok(ResponseEvent::OutputItemDone( - aggregated_item, - )))); + // Return the first pending event now. + if let Some(ev) = this.pending.pop_front() { + return Poll::Ready(Some(Ok(ev))); + } } // Nothing aggregated – forward Completed directly. @@ -439,11 +504,25 @@ where // will never appear in a Chat Completions stream. continue; } - Poll::Ready(Some(Ok(ResponseEvent::OutputTextDelta(_)))) - | Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryDelta(_)))) => { - // Deltas are ignored here since aggregation waits for the - // final OutputItemDone. - continue; + Poll::Ready(Some(Ok(ResponseEvent::OutputTextDelta(delta)))) => { + // Always accumulate deltas so we can emit a final OutputItemDone at Completed. + this.cumulative.push_str(&delta); + if this.streaming_mode { + // In streaming mode, also forward the delta immediately. + return Poll::Ready(Some(Ok(ResponseEvent::OutputTextDelta(delta)))); + } else { + continue; + } + } + Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryDelta(delta)))) => { + // Always accumulate reasoning deltas so we can emit a final Reasoning item at Completed. + this.cumulative_reasoning.push_str(&delta); + if this.streaming_mode { + // In streaming mode, also forward the delta immediately. + return Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryDelta(delta)))); + } else { + continue; + } } } } @@ -475,9 +554,23 @@ pub(crate) trait AggregateStreamExt: Stream> + Size AggregatedChatStream { inner: self, cumulative: String::new(), - pending_completed: None, + cumulative_reasoning: String::new(), + pending: std::collections::VecDeque::new(), + streaming_mode: false, } } } impl AggregateStreamExt for T where T: Stream> + Sized {} + +impl AggregatedChatStream { + pub(crate) fn streaming_mode(inner: S) -> Self { + AggregatedChatStream { + inner, + cumulative: String::new(), + cumulative_reasoning: String::new(), + pending: std::collections::VecDeque::new(), + streaming_mode: true, + } + } +} diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index b9ea6b13f4..8685bc54d3 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -93,7 +93,13 @@ impl ModelClient { // Wrap it with the aggregation adapter so callers see *only* // the final assistant message per turn (matching the // behaviour of the Responses API). - let mut aggregated = response_stream.aggregate(); + let mut aggregated = if self.config.show_reasoning_content + && !self.config.hide_agent_reasoning + { + crate::chat_completions::AggregatedChatStream::streaming_mode(response_stream) + } else { + response_stream.aggregate() + }; // Bridge the aggregated stream back into a standard // `ResponseStream` by forwarding events through a channel. @@ -438,7 +444,7 @@ async fn process_sse( } } } - "response.reasoning_summary_text.delta" => { + "response.reasoning_summary_text.delta" | "response.reasoning_text.delta" => { if let Some(delta) = event.delta { let event = ResponseEvent::ReasoningSummaryDelta(delta); if tx_event.send(Ok(event)).await.is_err() { diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 568d87c4a8..18bcf6261d 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -56,6 +56,7 @@ use crate::mcp_tool_call::handle_mcp_tool_call; 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; @@ -64,6 +65,7 @@ use crate::plan_tool::handle_update_plan; use crate::project_doc::get_user_instructions; use crate::protocol::AgentMessageDeltaEvent; use crate::protocol::AgentMessageEvent; +use crate::protocol::AgentReasoningContentEvent; use crate::protocol::AgentReasoningDeltaEvent; use crate::protocol::AgentReasoningEvent; use crate::protocol::ApplyPatchApprovalRequestEvent; @@ -227,6 +229,8 @@ pub(crate) struct Session { state: Mutex, codex_linux_sandbox_exe: Option, user_shell: shell::Shell, + show_reasoning_content: bool, + hide_agent_reasoning: bool, } impl Session { @@ -822,6 +826,8 @@ async fn submission_loop( codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), disable_response_storage, user_shell: default_shell, + show_reasoning_content: config.show_reasoning_content, + hide_agent_reasoning: config.hide_agent_reasoning, })); // Patch restored state into the newly created session. @@ -1132,6 +1138,7 @@ async fn run_task(sess: Arc, sub_id: String, input: Vec) { ResponseItem::Reasoning { id, summary, + content, encrypted_content, }, None, @@ -1139,6 +1146,7 @@ async fn run_task(sess: Arc, sub_id: String, input: Vec) { items_to_record_in_conversation_history.push(ResponseItem::Reasoning { id: id.clone(), summary: summary.clone(), + content: content.clone(), encrypted_content: encrypted_content.clone(), }); } @@ -1381,11 +1389,13 @@ async fn try_run_turn( sess.tx_event.send(event).await.ok(); } ResponseEvent::ReasoningSummaryDelta(delta) => { - let event = Event { - id: sub_id.to_string(), - msg: EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { delta }), - }; - sess.tx_event.send(event).await.ok(); + if !sess.hide_agent_reasoning { + let event = Event { + id: sub_id.to_string(), + msg: EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { delta }), + }; + sess.tx_event.send(event).await.ok(); + } } } } @@ -1493,16 +1503,36 @@ async fn handle_response_item( } None } - ResponseItem::Reasoning { summary, .. } => { - for item in summary { - let text = match item { - ReasoningItemReasoningSummary::SummaryText { text } => text, - }; - let event = Event { - id: sub_id.to_string(), - msg: EventMsg::AgentReasoning(AgentReasoningEvent { text }), - }; - sess.tx_event.send(event).await.ok(); + ResponseItem::Reasoning { + id: _, + summary, + content, + encrypted_content: _, + } => { + if !sess.hide_agent_reasoning { + for item in summary { + let text = match item { + ReasoningItemReasoningSummary::SummaryText { text } => text, + }; + let event = Event { + id: sub_id.to_string(), + msg: EventMsg::AgentReasoning(AgentReasoningEvent { text }), + }; + sess.tx_event.send(event).await.ok(); + } + } + if !sess.hide_agent_reasoning && sess.show_reasoning_content && content.is_some() { + let content = content.unwrap(); + for item in content { + let text = match item { + ReasoningItemContent::ReasoningText { text } => text, + }; + let event = Event { + id: sub_id.to_string(), + msg: EventMsg::AgentReasoningContent(AgentReasoningContentEvent { text }), + }; + sess.tx_event.send(event).await.ok(); + } } None } diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index b43dc56ba0..3277ca08e9 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -57,6 +57,10 @@ pub struct Config { /// users are only interested in the final agent responses. pub hide_agent_reasoning: bool, + /// When `true`, the raw chain-of-thought text from reasoning events will be + /// displayed in the UI in addition to the reasoning summaries. + pub show_reasoning_content: bool, + /// Disable server-side response storage (sends the full conversation /// context with every request). Currently necessary for OpenAI customers /// who have opted into Zero Data Retention (ZDR). @@ -325,6 +329,10 @@ pub struct ConfigToml { /// UI/output. Defaults to `false`. pub hide_agent_reasoning: Option, + /// When set to `true`, raw chain-of-thought text from reasoning events will + /// be shown in the UI. + pub show_reasoning_content: Option, + pub model_reasoning_effort: Option, pub model_reasoning_summary: Option, @@ -488,6 +496,19 @@ impl Config { Self::get_base_instructions(experimental_instructions_path, &resolved_cwd)?; let base_instructions = base_instructions.or(file_base_instructions); + // Resolve hide/show reasoning flags with consistent precedence: + // if hide is true, force show_reasoning_content to false. + let hide_agent_reasoning_val = cfg.hide_agent_reasoning.unwrap_or(false); + let show_reasoning_content_val = if hide_agent_reasoning_val { + false + } else { + cfg.show_reasoning_content.unwrap_or(false) + }; + + if cfg.hide_agent_reasoning == Some(true) && cfg.show_reasoning_content == Some(true) { + tracing::warn!("Ignoring show_reasoning_content because hide_agent_reasoning is true"); + } + let config = Self { model, model_context_window, @@ -517,7 +538,8 @@ impl Config { tui: cfg.tui.unwrap_or_default(), codex_linux_sandbox_exe, - hide_agent_reasoning: cfg.hide_agent_reasoning.unwrap_or(false), + hide_agent_reasoning: hide_agent_reasoning_val, + show_reasoning_content: show_reasoning_content_val, model_reasoning_effort: config_profile .model_reasoning_effort .or(cfg.model_reasoning_effort) @@ -891,6 +913,7 @@ disable_response_storage = true tui: Tui::default(), codex_linux_sandbox_exe: None, hide_agent_reasoning: false, + show_reasoning_content: false, model_reasoning_effort: ReasoningEffort::High, model_reasoning_summary: ReasoningSummary::Detailed, model_supports_reasoning_summaries: false, @@ -941,6 +964,7 @@ disable_response_storage = true tui: Tui::default(), codex_linux_sandbox_exe: None, hide_agent_reasoning: false, + show_reasoning_content: false, model_reasoning_effort: ReasoningEffort::default(), model_reasoning_summary: ReasoningSummary::default(), model_supports_reasoning_summaries: false, @@ -1006,6 +1030,7 @@ disable_response_storage = true tui: Tui::default(), codex_linux_sandbox_exe: None, hide_agent_reasoning: false, + show_reasoning_content: false, model_reasoning_effort: ReasoningEffort::default(), model_reasoning_summary: ReasoningSummary::default(), model_supports_reasoning_summaries: false, diff --git a/codex-rs/core/src/models.rs b/codex-rs/core/src/models.rs index 166404915a..98d8727e77 100644 --- a/codex-rs/core/src/models.rs +++ b/codex-rs/core/src/models.rs @@ -45,6 +45,8 @@ pub enum ResponseItem { Reasoning { id: String, summary: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + content: Option>, encrypted_content: Option, }, LocalShellCall { @@ -136,6 +138,12 @@ pub enum ReasoningItemReasoningSummary { SummaryText { text: String }, } +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum ReasoningItemContent { + ReasoningText { text: String }, +} + impl From> for ResponseInputItem { fn from(items: Vec) -> Self { Self::Message { diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index 82591a2c78..1e0733628c 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -359,6 +359,9 @@ pub enum EventMsg { /// Agent reasoning delta event from agent. AgentReasoningDelta(AgentReasoningDeltaEvent), + /// Raw chain-of-thought from agent. + AgentReasoningContent(AgentReasoningContentEvent), + /// Ack the client's configure message. SessionConfigured(SessionConfiguredEvent), @@ -464,6 +467,11 @@ pub struct AgentReasoningEvent { pub text: String, } +#[derive(Debug, Clone, Deserialize, Serialize)] +pub struct AgentReasoningContentEvent { + pub text: String, +} + #[derive(Debug, Clone, Deserialize, Serialize)] pub struct AgentReasoningDeltaEvent { pub delta: String, 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 7703c138fc..1f8fe3c031 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -4,6 +4,7 @@ use codex_core::config::Config; use codex_core::plan_tool::UpdatePlanArgs; use codex_core::protocol::AgentMessageDeltaEvent; use codex_core::protocol::AgentMessageEvent; +use codex_core::protocol::AgentReasoningContentEvent; use codex_core::protocol::AgentReasoningDeltaEvent; use codex_core::protocol::BackgroundEventEvent; use codex_core::protocol::ErrorEvent; @@ -203,6 +204,14 @@ impl EventProcessor for EventProcessorWithHumanOutput { #[allow(clippy::expect_used)] std::io::stdout().flush().expect("could not flush stdout"); } + EventMsg::AgentReasoningContent(AgentReasoningContentEvent { text }) => { + if !self.show_agent_reasoning { + return CodexStatus::Running; + } + print!("{text}"); + #[allow(clippy::expect_used)] + std::io::stdout().flush().expect("could not flush stdout"); + } EventMsg::AgentMessage(AgentMessageEvent { message }) => { // if answer_started is false, this means we haven't received any // delta. Thus, we need to print the message as a new answer. diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 205dfa4631..3d32d8de52 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -252,7 +252,8 @@ async fn run_codex_tool_session_inner( EventMsg::AgentMessage(AgentMessageEvent { .. }) => { // TODO: think how we want to support this in the MCP } - EventMsg::TaskStarted + EventMsg::AgentReasoningContent(_) + | EventMsg::TaskStarted | EventMsg::TokenCount(_) | EventMsg::AgentReasoning(_) | EventMsg::McpToolCallBegin(_) diff --git a/codex-rs/mcp-server/src/conversation_loop.rs b/codex-rs/mcp-server/src/conversation_loop.rs index 1db39a2306..5b95c313f4 100644 --- a/codex-rs/mcp-server/src/conversation_loop.rs +++ b/codex-rs/mcp-server/src/conversation_loop.rs @@ -90,7 +90,8 @@ pub async fn run_conversation_loop( EventMsg::AgentMessage(AgentMessageEvent { .. }) => { // TODO: think how we want to support this in the MCP } - EventMsg::TaskStarted + EventMsg::AgentReasoningContent(_) + | EventMsg::TaskStarted | EventMsg::TokenCount(_) | EventMsg::AgentReasoning(_) | EventMsg::McpToolCallBegin(_) From 1a33de34b06eea9731ae27d2a2ab532af586e782 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 4 Aug 2025 16:56:52 -0700 Subject: [PATCH 11/12] unify flag --- codex-rs/core/src/client.rs | 4 +--- codex-rs/core/src/codex.rs | 4 +--- codex-rs/core/src/config.rs | 21 --------------------- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 8685bc54d3..fd530e0c6d 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -93,9 +93,7 @@ impl ModelClient { // Wrap it with the aggregation adapter so callers see *only* // the final assistant message per turn (matching the // behaviour of the Responses API). - let mut aggregated = if self.config.show_reasoning_content - && !self.config.hide_agent_reasoning - { + let mut aggregated = if !self.config.hide_agent_reasoning { crate::chat_completions::AggregatedChatStream::streaming_mode(response_stream) } else { response_stream.aggregate() diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 18bcf6261d..caebaed233 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -229,7 +229,6 @@ pub(crate) struct Session { state: Mutex, codex_linux_sandbox_exe: Option, user_shell: shell::Shell, - show_reasoning_content: bool, hide_agent_reasoning: bool, } @@ -826,7 +825,6 @@ async fn submission_loop( codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), disable_response_storage, user_shell: default_shell, - show_reasoning_content: config.show_reasoning_content, hide_agent_reasoning: config.hide_agent_reasoning, })); @@ -1521,7 +1519,7 @@ async fn handle_response_item( sess.tx_event.send(event).await.ok(); } } - if !sess.hide_agent_reasoning && sess.show_reasoning_content && content.is_some() { + if !sess.hide_agent_reasoning && content.is_some() { let content = content.unwrap(); for item in content { let text = match item { diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 3277ca08e9..302c468b66 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -57,10 +57,6 @@ pub struct Config { /// users are only interested in the final agent responses. pub hide_agent_reasoning: bool, - /// When `true`, the raw chain-of-thought text from reasoning events will be - /// displayed in the UI in addition to the reasoning summaries. - pub show_reasoning_content: bool, - /// Disable server-side response storage (sends the full conversation /// context with every request). Currently necessary for OpenAI customers /// who have opted into Zero Data Retention (ZDR). @@ -329,10 +325,6 @@ pub struct ConfigToml { /// UI/output. Defaults to `false`. pub hide_agent_reasoning: Option, - /// When set to `true`, raw chain-of-thought text from reasoning events will - /// be shown in the UI. - pub show_reasoning_content: Option, - pub model_reasoning_effort: Option, pub model_reasoning_summary: Option, @@ -499,15 +491,6 @@ impl Config { // Resolve hide/show reasoning flags with consistent precedence: // if hide is true, force show_reasoning_content to false. let hide_agent_reasoning_val = cfg.hide_agent_reasoning.unwrap_or(false); - let show_reasoning_content_val = if hide_agent_reasoning_val { - false - } else { - cfg.show_reasoning_content.unwrap_or(false) - }; - - if cfg.hide_agent_reasoning == Some(true) && cfg.show_reasoning_content == Some(true) { - tracing::warn!("Ignoring show_reasoning_content because hide_agent_reasoning is true"); - } let config = Self { model, @@ -539,7 +522,6 @@ impl Config { codex_linux_sandbox_exe, hide_agent_reasoning: hide_agent_reasoning_val, - show_reasoning_content: show_reasoning_content_val, model_reasoning_effort: config_profile .model_reasoning_effort .or(cfg.model_reasoning_effort) @@ -913,7 +895,6 @@ disable_response_storage = true tui: Tui::default(), codex_linux_sandbox_exe: None, hide_agent_reasoning: false, - show_reasoning_content: false, model_reasoning_effort: ReasoningEffort::High, model_reasoning_summary: ReasoningSummary::Detailed, model_supports_reasoning_summaries: false, @@ -964,7 +945,6 @@ disable_response_storage = true tui: Tui::default(), codex_linux_sandbox_exe: None, hide_agent_reasoning: false, - show_reasoning_content: false, model_reasoning_effort: ReasoningEffort::default(), model_reasoning_summary: ReasoningSummary::default(), model_supports_reasoning_summaries: false, @@ -1030,7 +1010,6 @@ disable_response_storage = true tui: Tui::default(), codex_linux_sandbox_exe: None, hide_agent_reasoning: false, - show_reasoning_content: false, model_reasoning_effort: ReasoningEffort::default(), model_reasoning_summary: ReasoningSummary::default(), model_supports_reasoning_summaries: false, From e38ce39c514d035d0638c82c85d8fe1c4e120fc1 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 4 Aug 2025 17:03:24 -0700 Subject: [PATCH 12/12] Revert to 3f13ebce10209ab3645f51e7606892b3fd71d47e without rewriting history. Wrong merge --- codex-rs/core/src/chat_completions.rs | 163 ++++-------------- codex-rs/core/src/client.rs | 8 +- codex-rs/core/src/codex.rs | 58 ++----- codex-rs/core/src/config.rs | 6 +- codex-rs/core/src/models.rs | 8 - codex-rs/core/src/protocol.rs | 8 - .../src/event_processor_with_human_output.rs | 9 - codex-rs/mcp-server/src/codex_tool_runner.rs | 3 +- codex-rs/mcp-server/src/conversation_loop.rs | 3 +- 9 files changed, 55 insertions(+), 211 deletions(-) diff --git a/codex-rs/core/src/chat_completions.rs b/codex-rs/core/src/chat_completions.rs index d1b8338987..5ede774b1c 100644 --- a/codex-rs/core/src/chat_completions.rs +++ b/codex-rs/core/src/chat_completions.rs @@ -207,7 +207,6 @@ async fn process_chat_sse( } let mut fn_call_state = FunctionCallState::default(); - let mut assistant_text = String::new(); loop { let sse = match timeout(idle_timeout, stream.next()).await { @@ -255,42 +254,21 @@ async fn process_chat_sse( let choice_opt = chunk.get("choices").and_then(|c| c.get(0)); if let Some(choice) = choice_opt { - // Handle assistant content tokens as streaming deltas. + // Handle assistant content tokens. if let Some(content) = choice .get("delta") .and_then(|d| d.get("content")) .and_then(|c| c.as_str()) { - if !content.is_empty() { - assistant_text.push_str(content); - let _ = tx_event - .send(Ok(ResponseEvent::OutputTextDelta(content.to_string()))) - .await; - } - } + let item = ResponseItem::Message { + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { + text: content.to_string(), + }], + id: None, + }; - // Forward any reasoning/thinking deltas if present. - if let Some(reasoning) = choice - .get("delta") - .and_then(|d| d.get("reasoning")) - .and_then(|c| c.as_str()) - { - let _ = tx_event - .send(Ok(ResponseEvent::ReasoningSummaryDelta( - reasoning.to_string(), - ))) - .await; - } - if let Some(reasoning_content) = choice - .get("delta") - .and_then(|d| d.get("reasoning_content")) - .and_then(|c| c.as_str()) - { - let _ = tx_event - .send(Ok(ResponseEvent::ReasoningSummaryDelta( - reasoning_content.to_string(), - ))) - .await; + let _ = tx_event.send(Ok(ResponseEvent::OutputItemDone(item))).await; } // Handle streaming function / tool calls. @@ -339,18 +317,7 @@ async fn process_chat_sse( let _ = tx_event.send(Ok(ResponseEvent::OutputItemDone(item))).await; } "stop" => { - // Regular turn without tool-call. Emit the final assistant message - // as a single OutputItemDone so non-delta consumers see the result. - if !assistant_text.is_empty() { - let item = ResponseItem::Message { - role: "assistant".to_string(), - content: vec![ContentItem::OutputText { - text: std::mem::take(&mut assistant_text), - }], - id: None, - }; - let _ = tx_event.send(Ok(ResponseEvent::OutputItemDone(item))).await; - } + // Regular turn without tool-call. } _ => {} } @@ -391,10 +358,7 @@ async fn process_chat_sse( pub(crate) struct AggregatedChatStream { inner: S, cumulative: String, - cumulative_reasoning: String, - pending: std::collections::VecDeque, - // When true, do not emit a cumulative assistant message at Completed. - streaming_mode: bool, + pending_completed: Option, } impl Stream for AggregatedChatStream @@ -406,8 +370,8 @@ where fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { let this = self.get_mut(); - // First, flush any buffered events from the previous call. - if let Some(ev) = this.pending.pop_front() { + // First, flush any buffered Completed event from the previous call. + if let Some(ev) = this.pending_completed.take() { return Poll::Ready(Some(Ok(ev))); } @@ -424,21 +388,16 @@ where let is_assistant_delta = matches!(&item, crate::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() { - if let crate::models::ResponseItem::Message { content, .. } = &item { - if let Some(text) = content.iter().find_map(|c| match c { - crate::models::ContentItem::OutputText { text } => Some(text), - _ => None, - }) { - this.cumulative.push_str(text); - } + if let crate::models::ResponseItem::Message { content, .. } = &item { + if let Some(text) = content.iter().find_map(|c| match c { + crate::models::ContentItem::OutputText { text } => Some(text), + _ => None, + }) { + this.cumulative.push_str(text); } } - // Swallow assistant message here; emit on Completed. + // Swallow partial assistant chunk; keep polling. continue; } @@ -449,48 +408,24 @@ where response_id, token_usage, }))) => { - // Build any aggregated items in the correct order: Reasoning first, then Message. - let mut emitted_any = false; - - if !this.cumulative_reasoning.is_empty() { - let aggregated_reasoning = crate::models::ResponseItem::Reasoning { - id: String::new(), - summary: vec![ - crate::models::ReasoningItemReasoningSummary::SummaryText { - text: std::mem::take(&mut this.cumulative_reasoning), - }, - ], - content: None, - 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_item = crate::models::ResponseItem::Message { id: None, role: "assistant".to_string(), content: vec![crate::models::ContentItem::OutputText { text: std::mem::take(&mut this.cumulative), }], }; - this.pending - .push_back(ResponseEvent::OutputItemDone(aggregated_message)); - emitted_any = true; - } - // Always emit Completed last when anything was aggregated. - if emitted_any { - this.pending.push_back(ResponseEvent::Completed { - response_id: response_id.clone(), - token_usage: token_usage.clone(), + // Buffer Completed so it is returned *after* the aggregated message. + this.pending_completed = Some(ResponseEvent::Completed { + response_id, + token_usage, }); - // Return the first pending event now. - if let Some(ev) = this.pending.pop_front() { - return Poll::Ready(Some(Ok(ev))); - } + + return Poll::Ready(Some(Ok(ResponseEvent::OutputItemDone( + aggregated_item, + )))); } // Nothing aggregated – forward Completed directly. @@ -504,25 +439,11 @@ where // will never appear in a Chat Completions stream. continue; } - Poll::Ready(Some(Ok(ResponseEvent::OutputTextDelta(delta)))) => { - // Always accumulate deltas so we can emit a final OutputItemDone at Completed. - this.cumulative.push_str(&delta); - if this.streaming_mode { - // In streaming mode, also forward the delta immediately. - return Poll::Ready(Some(Ok(ResponseEvent::OutputTextDelta(delta)))); - } else { - continue; - } - } - Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryDelta(delta)))) => { - // Always accumulate reasoning deltas so we can emit a final Reasoning item at Completed. - this.cumulative_reasoning.push_str(&delta); - if this.streaming_mode { - // In streaming mode, also forward the delta immediately. - return Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryDelta(delta)))); - } else { - continue; - } + Poll::Ready(Some(Ok(ResponseEvent::OutputTextDelta(_)))) + | Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryDelta(_)))) => { + // Deltas are ignored here since aggregation waits for the + // final OutputItemDone. + continue; } } } @@ -554,23 +475,9 @@ pub(crate) trait AggregateStreamExt: Stream> + Size AggregatedChatStream { inner: self, cumulative: String::new(), - cumulative_reasoning: String::new(), - pending: std::collections::VecDeque::new(), - streaming_mode: false, + pending_completed: None, } } } impl AggregateStreamExt for T where T: Stream> + Sized {} - -impl AggregatedChatStream { - pub(crate) fn streaming_mode(inner: S) -> Self { - AggregatedChatStream { - inner, - cumulative: String::new(), - cumulative_reasoning: String::new(), - pending: std::collections::VecDeque::new(), - streaming_mode: true, - } - } -} diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index fd530e0c6d..b9ea6b13f4 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -93,11 +93,7 @@ impl ModelClient { // Wrap it with the aggregation adapter so callers see *only* // the final assistant message per turn (matching the // behaviour of the Responses API). - let mut aggregated = if !self.config.hide_agent_reasoning { - crate::chat_completions::AggregatedChatStream::streaming_mode(response_stream) - } else { - response_stream.aggregate() - }; + let mut aggregated = response_stream.aggregate(); // Bridge the aggregated stream back into a standard // `ResponseStream` by forwarding events through a channel. @@ -442,7 +438,7 @@ async fn process_sse( } } } - "response.reasoning_summary_text.delta" | "response.reasoning_text.delta" => { + "response.reasoning_summary_text.delta" => { if let Some(delta) = event.delta { let event = ResponseEvent::ReasoningSummaryDelta(delta); if tx_event.send(Ok(event)).await.is_err() { diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index caebaed233..568d87c4a8 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -56,7 +56,6 @@ use crate::mcp_tool_call::handle_mcp_tool_call; 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; @@ -65,7 +64,6 @@ use crate::plan_tool::handle_update_plan; use crate::project_doc::get_user_instructions; use crate::protocol::AgentMessageDeltaEvent; use crate::protocol::AgentMessageEvent; -use crate::protocol::AgentReasoningContentEvent; use crate::protocol::AgentReasoningDeltaEvent; use crate::protocol::AgentReasoningEvent; use crate::protocol::ApplyPatchApprovalRequestEvent; @@ -229,7 +227,6 @@ pub(crate) struct Session { state: Mutex, codex_linux_sandbox_exe: Option, user_shell: shell::Shell, - hide_agent_reasoning: bool, } impl Session { @@ -825,7 +822,6 @@ async fn submission_loop( codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), disable_response_storage, user_shell: default_shell, - hide_agent_reasoning: config.hide_agent_reasoning, })); // Patch restored state into the newly created session. @@ -1136,7 +1132,6 @@ async fn run_task(sess: Arc, sub_id: String, input: Vec) { ResponseItem::Reasoning { id, summary, - content, encrypted_content, }, None, @@ -1144,7 +1139,6 @@ async fn run_task(sess: Arc, sub_id: String, input: Vec) { items_to_record_in_conversation_history.push(ResponseItem::Reasoning { id: id.clone(), summary: summary.clone(), - content: content.clone(), encrypted_content: encrypted_content.clone(), }); } @@ -1387,13 +1381,11 @@ async fn try_run_turn( sess.tx_event.send(event).await.ok(); } ResponseEvent::ReasoningSummaryDelta(delta) => { - if !sess.hide_agent_reasoning { - let event = Event { - id: sub_id.to_string(), - msg: EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { delta }), - }; - sess.tx_event.send(event).await.ok(); - } + let event = Event { + id: sub_id.to_string(), + msg: EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { delta }), + }; + sess.tx_event.send(event).await.ok(); } } } @@ -1501,36 +1493,16 @@ async fn handle_response_item( } None } - ResponseItem::Reasoning { - id: _, - summary, - content, - encrypted_content: _, - } => { - if !sess.hide_agent_reasoning { - for item in summary { - let text = match item { - ReasoningItemReasoningSummary::SummaryText { text } => text, - }; - let event = Event { - id: sub_id.to_string(), - msg: EventMsg::AgentReasoning(AgentReasoningEvent { text }), - }; - sess.tx_event.send(event).await.ok(); - } - } - if !sess.hide_agent_reasoning && content.is_some() { - let content = content.unwrap(); - for item in content { - let text = match item { - ReasoningItemContent::ReasoningText { text } => text, - }; - let event = Event { - id: sub_id.to_string(), - msg: EventMsg::AgentReasoningContent(AgentReasoningContentEvent { text }), - }; - sess.tx_event.send(event).await.ok(); - } + ResponseItem::Reasoning { summary, .. } => { + for item in summary { + let text = match item { + ReasoningItemReasoningSummary::SummaryText { text } => text, + }; + let event = Event { + id: sub_id.to_string(), + msg: EventMsg::AgentReasoning(AgentReasoningEvent { text }), + }; + sess.tx_event.send(event).await.ok(); } None } diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 302c468b66..b43dc56ba0 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -488,10 +488,6 @@ impl Config { Self::get_base_instructions(experimental_instructions_path, &resolved_cwd)?; let base_instructions = base_instructions.or(file_base_instructions); - // Resolve hide/show reasoning flags with consistent precedence: - // if hide is true, force show_reasoning_content to false. - let hide_agent_reasoning_val = cfg.hide_agent_reasoning.unwrap_or(false); - let config = Self { model, model_context_window, @@ -521,7 +517,7 @@ impl Config { tui: cfg.tui.unwrap_or_default(), codex_linux_sandbox_exe, - hide_agent_reasoning: hide_agent_reasoning_val, + hide_agent_reasoning: cfg.hide_agent_reasoning.unwrap_or(false), model_reasoning_effort: config_profile .model_reasoning_effort .or(cfg.model_reasoning_effort) diff --git a/codex-rs/core/src/models.rs b/codex-rs/core/src/models.rs index 98d8727e77..166404915a 100644 --- a/codex-rs/core/src/models.rs +++ b/codex-rs/core/src/models.rs @@ -45,8 +45,6 @@ pub enum ResponseItem { Reasoning { id: String, summary: Vec, - #[serde(default, skip_serializing_if = "Option::is_none")] - content: Option>, encrypted_content: Option, }, LocalShellCall { @@ -138,12 +136,6 @@ pub enum ReasoningItemReasoningSummary { SummaryText { text: String }, } -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(tag = "type", rename_all = "snake_case")] -pub enum ReasoningItemContent { - ReasoningText { text: String }, -} - impl From> for ResponseInputItem { fn from(items: Vec) -> Self { Self::Message { diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index 1e0733628c..82591a2c78 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -359,9 +359,6 @@ pub enum EventMsg { /// Agent reasoning delta event from agent. AgentReasoningDelta(AgentReasoningDeltaEvent), - /// Raw chain-of-thought from agent. - AgentReasoningContent(AgentReasoningContentEvent), - /// Ack the client's configure message. SessionConfigured(SessionConfiguredEvent), @@ -467,11 +464,6 @@ pub struct AgentReasoningEvent { pub text: String, } -#[derive(Debug, Clone, Deserialize, Serialize)] -pub struct AgentReasoningContentEvent { - pub text: String, -} - #[derive(Debug, Clone, Deserialize, Serialize)] pub struct AgentReasoningDeltaEvent { pub delta: String, 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 1f8fe3c031..7703c138fc 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -4,7 +4,6 @@ use codex_core::config::Config; use codex_core::plan_tool::UpdatePlanArgs; use codex_core::protocol::AgentMessageDeltaEvent; use codex_core::protocol::AgentMessageEvent; -use codex_core::protocol::AgentReasoningContentEvent; use codex_core::protocol::AgentReasoningDeltaEvent; use codex_core::protocol::BackgroundEventEvent; use codex_core::protocol::ErrorEvent; @@ -204,14 +203,6 @@ impl EventProcessor for EventProcessorWithHumanOutput { #[allow(clippy::expect_used)] std::io::stdout().flush().expect("could not flush stdout"); } - EventMsg::AgentReasoningContent(AgentReasoningContentEvent { text }) => { - if !self.show_agent_reasoning { - return CodexStatus::Running; - } - print!("{text}"); - #[allow(clippy::expect_used)] - std::io::stdout().flush().expect("could not flush stdout"); - } EventMsg::AgentMessage(AgentMessageEvent { message }) => { // if answer_started is false, this means we haven't received any // delta. Thus, we need to print the message as a new answer. diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 3d32d8de52..205dfa4631 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -252,8 +252,7 @@ async fn run_codex_tool_session_inner( EventMsg::AgentMessage(AgentMessageEvent { .. }) => { // TODO: think how we want to support this in the MCP } - EventMsg::AgentReasoningContent(_) - | EventMsg::TaskStarted + EventMsg::TaskStarted | EventMsg::TokenCount(_) | EventMsg::AgentReasoning(_) | EventMsg::McpToolCallBegin(_) diff --git a/codex-rs/mcp-server/src/conversation_loop.rs b/codex-rs/mcp-server/src/conversation_loop.rs index 5b95c313f4..1db39a2306 100644 --- a/codex-rs/mcp-server/src/conversation_loop.rs +++ b/codex-rs/mcp-server/src/conversation_loop.rs @@ -90,8 +90,7 @@ pub async fn run_conversation_loop( EventMsg::AgentMessage(AgentMessageEvent { .. }) => { // TODO: think how we want to support this in the MCP } - EventMsg::AgentReasoningContent(_) - | EventMsg::TaskStarted + EventMsg::TaskStarted | EventMsg::TokenCount(_) | EventMsg::AgentReasoning(_) | EventMsg::McpToolCallBegin(_)