From d57f6510f754a64e6dc17ec75d319d4a7bf807ee Mon Sep 17 00:00:00 2001 From: jif-oai Date: Thu, 23 Oct 2025 18:51:08 +0100 Subject: [PATCH 01/11] V1 --- codex-rs/Cargo.lock | 2 + codex-rs/core/Cargo.toml | 2 + codex-rs/core/src/chat_completions.rs | 5 + codex-rs/core/src/codex.rs | 50 ++++++- codex-rs/core/src/conversation_history.rs | 8 +- codex-rs/core/src/rollout/policy.rs | 3 +- codex-rs/core/src/state/session.rs | 2 - codex-rs/core/src/tasks/ghost_snapshot.rs | 139 ++++++++++++++++++ codex-rs/core/src/tasks/mod.rs | 2 + codex-rs/core/src/tools/parallel.rs | 10 ++ .../core/tests/suite/compact_resume_fork.rs | 31 +++- codex-rs/protocol/src/models.rs | 6 + 12 files changed, 251 insertions(+), 9 deletions(-) create mode 100644 codex-rs/core/src/tasks/ghost_snapshot.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 97144613d6..33d6894409 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1061,10 +1061,12 @@ dependencies = [ "codex-apply-patch", "codex-async-utils", "codex-file-search", + "codex-git-tooling", "codex-otel", "codex-protocol", "codex-rmcp-client", "codex-utils-pty", + "codex-utils-readiness", "codex-utils-string", "core-foundation 0.9.4", "core_test_support", diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 36d5358605..2684bff5f0 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -24,10 +24,12 @@ codex-apply-patch = { workspace = true } codex-file-search = { workspace = true } codex-otel = { workspace = true, features = ["otel"] } codex-protocol = { workspace = true } +codex-git-tooling = { workspace = true } codex-rmcp-client = { workspace = true } codex-async-utils = { workspace = true } codex-utils-string = { workspace = true } codex-utils-pty = { workspace = true } +codex-utils-readiness = { workspace = true } dirs = { workspace = true } dunce = { workspace = true } env-flags = { workspace = true } diff --git a/codex-rs/core/src/chat_completions.rs b/codex-rs/core/src/chat_completions.rs index 27937a48d0..36575594d6 100644 --- a/codex-rs/core/src/chat_completions.rs +++ b/codex-rs/core/src/chat_completions.rs @@ -75,6 +75,7 @@ pub(crate) async fn stream_chat_completions( ResponseItem::CustomToolCall { .. } => {} ResponseItem::CustomToolCallOutput { .. } => {} ResponseItem::WebSearchCall { .. } => {} + ResponseItem::GhostSnapshot { .. } => {} } } @@ -269,6 +270,10 @@ pub(crate) async fn stream_chat_completions( "content": output, })); } + ResponseItem::GhostSnapshot { .. } => { + // Ghost snapshots annotate history but are not sent to the model. + continue; + } ResponseItem::Reasoning { .. } | ResponseItem::WebSearchCall { .. } | ResponseItem::Other => { diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index b261b083ca..408bf415dc 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -16,6 +16,7 @@ use crate::user_notification::UserNotifier; use async_channel::Receiver; use async_channel::Sender; use codex_apply_patch::ApplyPatchAction; +use codex_git_tooling::GhostCommit; use codex_protocol::ConversationId; use codex_protocol::items::TurnItem; use codex_protocol::protocol::ConversationPathResponseEvent; @@ -104,6 +105,7 @@ use crate::state::TaskKind; use crate::tasks::CompactTask; use crate::tasks::RegularTask; use crate::tasks::ReviewTask; +use crate::tasks::spawn_ghost_snapshot_task; use crate::tools::ToolRouter; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::parallel::ToolCallRuntime; @@ -126,6 +128,8 @@ use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::InitialHistory; use codex_protocol::user_input::UserInput; +use codex_utils_readiness::Readiness; +use codex_utils_readiness::ReadinessFlag; pub mod compact; use self::compact::build_compacted_history; @@ -855,7 +859,7 @@ impl Session { /// Records input items: always append to conversation history and /// persist these response items to rollout. - async fn record_conversation_items(&self, items: &[ResponseItem]) { + pub(crate) async fn record_conversation_items(&self, items: &[ResponseItem]) { self.record_into_history(items).await; self.persist_rollout_response_items(items).await; } @@ -1035,6 +1039,33 @@ impl Session { self.send_event(turn_context, event).await; } + async fn maybe_start_ghost_snapshot( + self: &Arc, + turn_context: Arc, + ) -> Option> { + if turn_context.is_review_mode { + return None; + } + + let readiness = Arc::new(ReadinessFlag::new()); + let token = match readiness.subscribe().await { + Ok(token) => token, + Err(err) => { + warn!("failed to subscribe to ghost snapshot readiness: {err}"); + return None; + } + }; + + spawn_ghost_snapshot_task( + Arc::clone(self), + turn_context, + Arc::clone(&readiness), + token, + ); + + Some(readiness) + } + /// Returns the input if there was no task running to inject into pub async fn inject_input(&self, input: Vec) -> Result<(), Vec> { let mut active = self.active_turn.lock().await; @@ -1533,6 +1564,9 @@ pub(crate) async fn run_task( .await; } + let ghost_snapshot_gate = sess + .maybe_start_ghost_snapshot(Arc::clone(&turn_context)) + .await; let mut last_agent_message: Option = None; // 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. @@ -1588,6 +1622,7 @@ pub(crate) async fn run_task( Arc::clone(&turn_context), Arc::clone(&turn_diff_tracker), turn_input, + ghost_snapshot_gate.clone(), task_kind, cancellation_token.child_token(), ) @@ -1809,11 +1844,19 @@ fn parse_review_output_event(text: &str) -> ReviewOutputEvent { } } +fn filter_model_visible_history(input: Vec) -> Vec { + input + .into_iter() + .filter(|item| !matches!(item, ResponseItem::GhostSnapshot { .. })) + .collect() +} + async fn run_turn( sess: Arc, turn_context: Arc, turn_diff_tracker: SharedTurnDiffTracker, input: Vec, + ghost_snapshot_gate: Option>, task_kind: TaskKind, cancellation_token: CancellationToken, ) -> CodexResult { @@ -1829,7 +1872,7 @@ async fn run_turn( .supports_parallel_tool_calls; let parallel_tool_calls = model_supports_parallel; let prompt = Prompt { - input, + input: filter_model_visible_history(input), tools: router.specs(), parallel_tool_calls, base_instructions_override: turn_context.base_instructions.clone(), @@ -1844,6 +1887,7 @@ async fn run_turn( Arc::clone(&turn_context), Arc::clone(&turn_diff_tracker), &prompt, + ghost_snapshot_gate.clone(), task_kind, cancellation_token.child_token(), ) @@ -1921,6 +1965,7 @@ async fn try_run_turn( turn_context: Arc, turn_diff_tracker: SharedTurnDiffTracker, prompt: &Prompt, + ghost_snapshot_gate: Option>, task_kind: TaskKind, cancellation_token: CancellationToken, ) -> CodexResult { @@ -1946,6 +1991,7 @@ async fn try_run_turn( Arc::clone(&sess), Arc::clone(&turn_context), Arc::clone(&turn_diff_tracker), + ghost_snapshot_gate.clone(), ); let mut output: FuturesOrdered>> = FuturesOrdered::new(); diff --git a/codex-rs/core/src/conversation_history.rs b/codex-rs/core/src/conversation_history.rs index 93234a4936..5a16be6b9c 100644 --- a/codex-rs/core/src/conversation_history.rs +++ b/codex-rs/core/src/conversation_history.rs @@ -1,5 +1,6 @@ use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseItem; +use std::ops::Deref; use tracing::error; /// Transcript of conversation history @@ -21,7 +22,9 @@ impl ConversationHistory { I::Item: std::ops::Deref, { for item in items { - if !is_api_message(&item) { + let item_ref = item.deref(); + let is_ghost_snapshot = matches!(item_ref, ResponseItem::GhostSnapshot { .. }); + if !is_api_message(item_ref) && !is_ghost_snapshot { continue; } @@ -146,6 +149,7 @@ impl ConversationHistory { | ResponseItem::WebSearchCall { .. } | ResponseItem::FunctionCallOutput { .. } | ResponseItem::CustomToolCallOutput { .. } + | ResponseItem::GhostSnapshot { .. } | ResponseItem::Other | ResponseItem::Message { .. } => { // nothing to do for these variants @@ -212,6 +216,7 @@ impl ConversationHistory { | ResponseItem::LocalShellCall { .. } | ResponseItem::Reasoning { .. } | ResponseItem::WebSearchCall { .. } + | ResponseItem::GhostSnapshot { .. } | ResponseItem::Other | ResponseItem::Message { .. } => { // nothing to do for these variants @@ -324,6 +329,7 @@ fn is_api_message(message: &ResponseItem) -> bool { | ResponseItem::LocalShellCall { .. } | ResponseItem::Reasoning { .. } | ResponseItem::WebSearchCall { .. } => true, + ResponseItem::GhostSnapshot { .. } => false, ResponseItem::Other => false, } } diff --git a/codex-rs/core/src/rollout/policy.rs b/codex-rs/core/src/rollout/policy.rs index fdf1f5ccb2..357d3d5adb 100644 --- a/codex-rs/core/src/rollout/policy.rs +++ b/codex-rs/core/src/rollout/policy.rs @@ -26,7 +26,8 @@ pub(crate) fn should_persist_response_item(item: &ResponseItem) -> bool { | ResponseItem::FunctionCallOutput { .. } | ResponseItem::CustomToolCall { .. } | ResponseItem::CustomToolCallOutput { .. } - | ResponseItem::WebSearchCall { .. } => true, + | ResponseItem::WebSearchCall { .. } + | ResponseItem::GhostSnapshot { .. } => true, ResponseItem::Other => false, } } diff --git a/codex-rs/core/src/state/session.rs b/codex-rs/core/src/state/session.rs index f8a58c3b2a..4f15fc6b46 100644 --- a/codex-rs/core/src/state/session.rs +++ b/codex-rs/core/src/state/session.rs @@ -79,6 +79,4 @@ impl SessionState { } } } - - // Pending input/approval moved to TurnState. } diff --git a/codex-rs/core/src/tasks/ghost_snapshot.rs b/codex-rs/core/src/tasks/ghost_snapshot.rs new file mode 100644 index 0000000000..1a097d0a6e --- /dev/null +++ b/codex-rs/core/src/tasks/ghost_snapshot.rs @@ -0,0 +1,139 @@ +use std::borrow::ToOwned; +use std::sync::Arc; + +use codex_git_tooling::CreateGhostCommitOptions; +use codex_git_tooling::GhostCommit; +use codex_git_tooling::GitToolingError; +use codex_git_tooling::create_ghost_commit; +use codex_protocol::models::ResponseItem; +use codex_utils_readiness::Readiness; +use codex_utils_readiness::ReadinessFlag; +use codex_utils_readiness::Token; +use tokio::task; +use tokio::task::JoinError; +use tracing::info; +use tracing::warn; + +use crate::codex::Session; +use crate::codex::TurnContext; + +pub(crate) fn spawn_ghost_snapshot_task( + session: Arc, + turn_context: Arc, + readiness: Arc, + token: Token, +) { + task::spawn(async move { + GhostSnapshotTask::new(session, turn_context, readiness, token) + .run() + .await; + }); +} + +struct GhostSnapshotTask { + session: Arc, + turn_context: Arc, + readiness: Arc, + token: Token, +} + +impl GhostSnapshotTask { + fn new( + session: Arc, + turn_context: Arc, + readiness: Arc, + token: Token, + ) -> Self { + Self { + session, + turn_context, + readiness, + token, + } + } + + async fn run(self) { + let repo_path = self.turn_context.cwd.clone(); + let snapshot = task::spawn_blocking(move || { + let options = CreateGhostCommitOptions::new(&repo_path); + create_ghost_commit(&options) + }) + .await; + + match snapshot { + Ok(Ok(commit)) => self.handle_success(commit).await, + Ok(Err(err)) => self.handle_git_error(err).await, + Err(err) => self.handle_task_failure(err).await, + } + + if let Err(err) = self.readiness.mark_ready(self.token).await { + warn!("failed to mark ghost snapshot ready: {err}"); + } + } + + async fn handle_success(&self, commit: GhostCommit) { + + self.session.record_conversation_items( + &[ResponseItem::GhostSnapshot { + commit_id: commit.id().to_string(), + parent: commit.parent().map(ToOwned::to_owned), + }] + ).await; + info!( + sub_id = self.turn_context.sub_id.as_str(), + commit_id = commit.id(), + "captured ghost snapshot" + ); + } + + async fn handle_git_error(&self, err: GitToolingError) { + warn!( + sub_id = self.turn_context.sub_id.as_str(), + "failed to capture ghost snapshot: {err}" + ); + let message = match err { + GitToolingError::NotAGitRepository { .. } => { + "Snapshots disabled: current directory is not a Git repository.".to_string() + } + _ => format!( + "Snapshots disabled after ghost snapshot error: {err}." + ), + }; + self.session + .notify_background_event(self.turn_context.as_ref(), message) + .await; + } + + async fn handle_task_failure(&self, err: JoinError) { + warn!( + sub_id = self.turn_context.sub_id.as_str(), + "ghost snapshot task failed: {err}" + ); + self.session + .notify_background_event( + self.turn_context.as_ref(), + "Failed to capture workspace snapshot due to an internal error.", + ) + .await; + } +} + +#[cfg(test)] +mod tests { + use super::ghost_snapshot_response_item; + use codex_git_tooling::GhostCommit; + use codex_protocol::models::ResponseItem; + + #[test] + fn ghost_snapshot_response_item_includes_commit_ids() { + let commit = GhostCommit::new("abc123".to_string(), Some("def456".to_string())); + let item = ghost_snapshot_response_item(&commit); + match item { + ResponseItem::GhostSnapshot { commit_id, parent } => { + assert_eq!(commit_id, "abc123"); + assert_eq!(parent, Some("def456".to_string())); + } + other => panic!("unexpected response item: {other:?}"), + } + } +} diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index 79527814e3..5c03e8278c 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -1,4 +1,5 @@ mod compact; +mod ghost_snapshot; mod regular; mod review; @@ -25,6 +26,7 @@ use crate::state::TaskKind; use codex_protocol::user_input::UserInput; pub(crate) use compact::CompactTask; +pub(crate) use ghost_snapshot::spawn_ghost_snapshot_task; pub(crate) use regular::RegularTask; pub(crate) use review::ReviewTask; diff --git a/codex-rs/core/src/tools/parallel.rs b/codex-rs/core/src/tools/parallel.rs index eae181c1c5..b266d1b9eb 100644 --- a/codex-rs/core/src/tools/parallel.rs +++ b/codex-rs/core/src/tools/parallel.rs @@ -12,6 +12,8 @@ use crate::tools::context::SharedTurnDiffTracker; use crate::tools::router::ToolCall; use crate::tools::router::ToolRouter; use codex_protocol::models::ResponseInputItem; +use codex_utils_readiness::Readiness; +use codex_utils_readiness::ReadinessFlag; pub(crate) struct ToolCallRuntime { router: Arc, @@ -19,6 +21,8 @@ pub(crate) struct ToolCallRuntime { turn_context: Arc, tracker: SharedTurnDiffTracker, parallel_execution: Arc>, + // Gate to wait before running the first tool call. + tool_gate: Option>, } impl ToolCallRuntime { @@ -27,6 +31,7 @@ impl ToolCallRuntime { session: Arc, turn_context: Arc, tracker: SharedTurnDiffTracker, + tool_gate: Option>, ) -> Self { Self { router, @@ -34,6 +39,7 @@ impl ToolCallRuntime { turn_context, tracker, parallel_execution: Arc::new(RwLock::new(())), + tool_gate, } } @@ -48,9 +54,13 @@ impl ToolCallRuntime { let turn = Arc::clone(&self.turn_context); let tracker = Arc::clone(&self.tracker); let lock = Arc::clone(&self.parallel_execution); + let readiness = self.tool_gate.clone(); let handle: AbortOnDropHandle> = AbortOnDropHandle::new(tokio::spawn(async move { + if let Some(flag) = readiness { + flag.wait_ready().await; + } let _guard = if supports_parallel { Either::Left(lock.read().await) } else { diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index 4261a30510..0a8482c981 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -42,6 +42,29 @@ fn network_disabled() -> bool { std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() } +fn filter_out_ghost_snapshot_entries(items: &[Value]) -> Vec { + items + .iter() + .filter(|item| !is_ghost_snapshot_message(item)) + .cloned() + .collect() +} + +fn is_ghost_snapshot_message(item: &Value) -> bool { + if item.get("type").and_then(Value::as_str) != Some("message") { + return false; + } + if item.get("role").and_then(Value::as_str) != Some("user") { + return false; + } + item.get("content") + .and_then(Value::as_array) + .and_then(|content| content.first()) + .and_then(|entry| entry.get("text")) + .and_then(Value::as_str) + .is_some_and(|text| text.trim_start().starts_with("")) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] /// Scenario: compact an initial conversation, resume it, fork one turn back, and /// ensure the model-visible history matches expectations at each request. @@ -557,13 +580,15 @@ async fn compact_resume_after_second_compaction_preserves_history() { let resume_input_array = input_after_resume .as_array() .expect("input after resume should be an array"); + let compact_filtered = filter_out_ghost_snapshot_entries(compact_input_array); + let resume_filtered = filter_out_ghost_snapshot_entries(resume_input_array); assert!( - compact_input_array.len() <= resume_input_array.len(), + compact_filtered.len() <= resume_filtered.len(), "after-resume input should have at least as many items as after-compact" ); assert_eq!( - compact_input_array.as_slice(), - &resume_input_array[..compact_input_array.len()] + compact_filtered.as_slice(), + &resume_filtered[..compact_filtered.len()] ); // hard coded test let prompt = requests[0]["instructions"] diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 4e99455bd2..02d557535e 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -116,6 +116,12 @@ pub enum ResponseItem { status: Option, action: WebSearchAction, }, + // Generated by the harness but considered exactly as a model response. + GhostSnapshot { + commit_id: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + parent: Option, + }, #[serde(other)] Other, } From c5a8030ee5603cc2683c74dbf8f7b9eb006ef264 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 24 Oct 2025 11:14:57 +0100 Subject: [PATCH 02/11] V2 --- codex-rs/core/src/codex.rs | 47 +++--- codex-rs/core/src/tasks/ghost_snapshot.rs | 180 ++++++++-------------- codex-rs/core/src/tasks/mod.rs | 2 +- codex-rs/core/src/tools/parallel.rs | 13 +- codex-rs/utils/readiness/src/lib.rs | 9 ++ 5 files changed, 100 insertions(+), 151 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 408bf415dc..52843bd51c 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -16,7 +16,6 @@ use crate::user_notification::UserNotifier; use async_channel::Receiver; use async_channel::Sender; use codex_apply_patch::ApplyPatchAction; -use codex_git_tooling::GhostCommit; use codex_protocol::ConversationId; use codex_protocol::items::TurnItem; use codex_protocol::protocol::ConversationPathResponseEvent; @@ -102,10 +101,10 @@ use crate::state::ActiveTurn; use crate::state::SessionServices; use crate::state::SessionState; use crate::state::TaskKind; -use crate::tasks::CompactTask; +use crate::tasks::{CompactTask, SessionTask, SessionTaskContext}; +use crate::tasks::GhostSnapshotTask; use crate::tasks::RegularTask; use crate::tasks::ReviewTask; -use crate::tasks::spawn_ghost_snapshot_task; use crate::tools::ToolRouter; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::parallel::ToolCallRuntime; @@ -273,6 +272,7 @@ pub(crate) struct TurnContext { pub(crate) is_review_mode: bool, pub(crate) final_output_json_schema: Option, pub(crate) codex_linux_sandbox_exe: Option, + pub(crate) tool_call_gate: Arc, } impl TurnContext { @@ -408,6 +408,7 @@ impl Session { is_review_mode: false, final_output_json_schema: None, codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), + tool_call_gate: Arc::new(ReadinessFlag::new()) } } @@ -1042,28 +1043,30 @@ impl Session { async fn maybe_start_ghost_snapshot( self: &Arc, turn_context: Arc, - ) -> Option> { + cancellation_token: CancellationToken, + ) { if turn_context.is_review_mode { - return None; + return; } - let readiness = Arc::new(ReadinessFlag::new()); - let token = match readiness.subscribe().await { + let token = match turn_context.tool_call_gate.subscribe().await { Ok(token) => token, Err(err) => { warn!("failed to subscribe to ghost snapshot readiness: {err}"); - return None; + return; } }; - spawn_ghost_snapshot_task( - Arc::clone(self), - turn_context, - Arc::clone(&readiness), + info!("spawning ghost snapshot task"); + let task = GhostSnapshotTask::new( token, ); - - Some(readiness) + Arc::new(task).run( + Arc::new(SessionTaskContext::new(self.clone())), + turn_context.clone(), + Vec::new(), + cancellation_token + ).await; } /// Returns the input if there was no task running to inject into @@ -1501,6 +1504,7 @@ async fn spawn_review_thread( is_review_mode: true, final_output_json_schema: None, codex_linux_sandbox_exe: parent_turn_context.codex_linux_sandbox_exe.clone(), + tool_call_gate: Arc::new(ReadinessFlag::new()) }; // Seed the child task with the review prompt as the initial user message. @@ -1564,9 +1568,7 @@ pub(crate) async fn run_task( .await; } - let ghost_snapshot_gate = sess - .maybe_start_ghost_snapshot(Arc::clone(&turn_context)) - .await; + sess.maybe_start_ghost_snapshot(Arc::clone(&turn_context), cancellation_token.child_token()).await; let mut last_agent_message: Option = None; // 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. @@ -1622,7 +1624,6 @@ pub(crate) async fn run_task( Arc::clone(&turn_context), Arc::clone(&turn_diff_tracker), turn_input, - ghost_snapshot_gate.clone(), task_kind, cancellation_token.child_token(), ) @@ -1856,7 +1857,6 @@ async fn run_turn( turn_context: Arc, turn_diff_tracker: SharedTurnDiffTracker, input: Vec, - ghost_snapshot_gate: Option>, task_kind: TaskKind, cancellation_token: CancellationToken, ) -> CodexResult { @@ -1887,7 +1887,6 @@ async fn run_turn( Arc::clone(&turn_context), Arc::clone(&turn_diff_tracker), &prompt, - ghost_snapshot_gate.clone(), task_kind, cancellation_token.child_token(), ) @@ -1965,7 +1964,6 @@ async fn try_run_turn( turn_context: Arc, turn_diff_tracker: SharedTurnDiffTracker, prompt: &Prompt, - ghost_snapshot_gate: Option>, task_kind: TaskKind, cancellation_token: CancellationToken, ) -> CodexResult { @@ -1991,7 +1989,6 @@ async fn try_run_turn( Arc::clone(&sess), Arc::clone(&turn_context), Arc::clone(&turn_diff_tracker), - ghost_snapshot_gate.clone(), ); let mut output: FuturesOrdered>> = FuturesOrdered::new(); @@ -2104,9 +2101,6 @@ async fn try_run_turn( response_id: _, token_usage, } => { - sess.update_token_usage_info(turn_context.as_ref(), token_usage.as_ref()) - .await; - let processed_items = output .try_collect() .or_cancel(&cancellation_token) @@ -2121,6 +2115,9 @@ async fn try_run_turn( sess.send_event(&turn_context, msg).await; } + sess.update_token_usage_info(turn_context.as_ref(), token_usage.as_ref()) + .await; + let result = TurnRunResult { processed_items, total_token_usage: token_usage.clone(), diff --git a/codex-rs/core/src/tasks/ghost_snapshot.rs b/codex-rs/core/src/tasks/ghost_snapshot.rs index 1a097d0a6e..5ce9b4d89a 100644 --- a/codex-rs/core/src/tasks/ghost_snapshot.rs +++ b/codex-rs/core/src/tasks/ghost_snapshot.rs @@ -1,139 +1,87 @@ use std::borrow::ToOwned; use std::sync::Arc; - +use async_trait::async_trait; +use crate::codex::TurnContext; +use crate::state::TaskKind; +use crate::tasks::SessionTask; +use crate::tasks::SessionTaskContext; use codex_git_tooling::CreateGhostCommitOptions; -use codex_git_tooling::GhostCommit; use codex_git_tooling::GitToolingError; use codex_git_tooling::create_ghost_commit; use codex_protocol::models::ResponseItem; +use codex_protocol::user_input::UserInput; use codex_utils_readiness::Readiness; -use codex_utils_readiness::ReadinessFlag; use codex_utils_readiness::Token; -use tokio::task; -use tokio::task::JoinError; +use tokio_util::sync::CancellationToken; use tracing::info; use tracing::warn; -use crate::codex::Session; -use crate::codex::TurnContext; - -pub(crate) fn spawn_ghost_snapshot_task( - session: Arc, - turn_context: Arc, - readiness: Arc, +pub(crate) struct GhostSnapshotTask { token: Token, -) { - task::spawn(async move { - GhostSnapshotTask::new(session, turn_context, readiness, token) - .run() - .await; - }); } -struct GhostSnapshotTask { - session: Arc, - turn_context: Arc, - readiness: Arc, - token: Token, -} - -impl GhostSnapshotTask { - fn new( - session: Arc, - turn_context: Arc, - readiness: Arc, - token: Token, - ) -> Self { - Self { - session, - turn_context, - readiness, - token, - } +#[async_trait] +impl SessionTask for GhostSnapshotTask { + fn kind(&self) -> TaskKind { + TaskKind::Regular } - async fn run(self) { - let repo_path = self.turn_context.cwd.clone(); - let snapshot = task::spawn_blocking(move || { + async fn run( + self: Arc, + session: Arc, + ctx: Arc, + _input: Vec, + cancellation_token: CancellationToken, // TODO handle cancellation token with a tokio select + ) -> Option { + tokio::task::spawn(async move { + let repo_path = ctx.cwd.clone(); let options = CreateGhostCommitOptions::new(&repo_path); - create_ghost_commit(&options) - }) - .await; - - match snapshot { - Ok(Ok(commit)) => self.handle_success(commit).await, - Ok(Err(err)) => self.handle_git_error(err).await, - Err(err) => self.handle_task_failure(err).await, - } - - if let Err(err) = self.readiness.mark_ready(self.token).await { - warn!("failed to mark ghost snapshot ready: {err}"); - } - } - - async fn handle_success(&self, commit: GhostCommit) { - - self.session.record_conversation_items( - &[ResponseItem::GhostSnapshot { - commit_id: commit.id().to_string(), - parent: commit.parent().map(ToOwned::to_owned), - }] - ).await; - info!( - sub_id = self.turn_context.sub_id.as_str(), - commit_id = commit.id(), - "captured ghost snapshot" - ); - } - - async fn handle_git_error(&self, err: GitToolingError) { - warn!( - sub_id = self.turn_context.sub_id.as_str(), - "failed to capture ghost snapshot: {err}" - ); - let message = match err { - GitToolingError::NotAGitRepository { .. } => { - "Snapshots disabled: current directory is not a Git repository.".to_string() + let ghost_commit = create_ghost_commit(&options); + info!("ghost snapshot blocking task finished"); + match ghost_commit { + Ok(ghost_commit) => { + session + .session + .record_conversation_items(&[ResponseItem::GhostSnapshot { + commit_id: ghost_commit.id().to_string(), + parent: ghost_commit.parent().map(ToOwned::to_owned), + }]) + .await; + info!("ghost commit captured: {}", ghost_commit.id()); + } + Err(err) => { + warn!( + sub_id = ctx.sub_id.as_str(), + "failed to capture ghost snapshot: {err}" + ); + let message = match err { + GitToolingError::NotAGitRepository { .. } => { + "Snapshots disabled: current directory is not a Git repository." + .to_string() + } + _ => format!("Snapshots disabled after ghost snapshot error: {err}."), + }; + session.session + .notify_background_event(&ctx, message) + .await; + } } - _ => format!( - "Snapshots disabled after ghost snapshot error: {err}." - ), - }; - self.session - .notify_background_event(self.turn_context.as_ref(), message) - .await; - } - - async fn handle_task_failure(&self, err: JoinError) { - warn!( - sub_id = self.turn_context.sub_id.as_str(), - "ghost snapshot task failed: {err}" - ); - self.session - .notify_background_event( - self.turn_context.as_ref(), - "Failed to capture workspace snapshot due to an internal error.", - ) - .await; + match ctx.tool_call_gate.mark_ready(self.token).await { + Ok(true) => info!("ghost snapshot gate marked ready"), + Ok(false) => warn!("ghost snapshot gate already ready"), + Err(err) => warn!("failed to mark ghost snapshot ready: {err}"), + } + }); + None } } -#[cfg(test)] -mod tests { - use super::ghost_snapshot_response_item; - use codex_git_tooling::GhostCommit; - use codex_protocol::models::ResponseItem; - - #[test] - fn ghost_snapshot_response_item_includes_commit_ids() { - let commit = GhostCommit::new("abc123".to_string(), Some("def456".to_string())); - let item = ghost_snapshot_response_item(&commit); - match item { - ResponseItem::GhostSnapshot { commit_id, parent } => { - assert_eq!(commit_id, "abc123"); - assert_eq!(parent, Some("def456".to_string())); - } - other => panic!("unexpected response item: {other:?}"), +impl GhostSnapshotTask { + pub(crate) fn new( + token: Token, + ) -> Self { + Self { + token, } } -} +} \ No newline at end of file diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index 5c03e8278c..a5afbca2e3 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -26,7 +26,7 @@ use crate::state::TaskKind; use codex_protocol::user_input::UserInput; pub(crate) use compact::CompactTask; -pub(crate) use ghost_snapshot::spawn_ghost_snapshot_task; +pub(crate) use ghost_snapshot::GhostSnapshotTask; pub(crate) use regular::RegularTask; pub(crate) use review::ReviewTask; diff --git a/codex-rs/core/src/tools/parallel.rs b/codex-rs/core/src/tools/parallel.rs index b266d1b9eb..2ca000d095 100644 --- a/codex-rs/core/src/tools/parallel.rs +++ b/codex-rs/core/src/tools/parallel.rs @@ -13,7 +13,6 @@ use crate::tools::router::ToolCall; use crate::tools::router::ToolRouter; use codex_protocol::models::ResponseInputItem; use codex_utils_readiness::Readiness; -use codex_utils_readiness::ReadinessFlag; pub(crate) struct ToolCallRuntime { router: Arc, @@ -21,8 +20,6 @@ pub(crate) struct ToolCallRuntime { turn_context: Arc, tracker: SharedTurnDiffTracker, parallel_execution: Arc>, - // Gate to wait before running the first tool call. - tool_gate: Option>, } impl ToolCallRuntime { @@ -31,7 +28,6 @@ impl ToolCallRuntime { session: Arc, turn_context: Arc, tracker: SharedTurnDiffTracker, - tool_gate: Option>, ) -> Self { Self { router, @@ -39,7 +35,6 @@ impl ToolCallRuntime { turn_context, tracker, parallel_execution: Arc::new(RwLock::new(())), - tool_gate, } } @@ -54,13 +49,13 @@ impl ToolCallRuntime { let turn = Arc::clone(&self.turn_context); let tracker = Arc::clone(&self.tracker); let lock = Arc::clone(&self.parallel_execution); - let readiness = self.tool_gate.clone(); + let readiness = self.turn_context.tool_call_gate.clone(); let handle: AbortOnDropHandle> = AbortOnDropHandle::new(tokio::spawn(async move { - if let Some(flag) = readiness { - flag.wait_ready().await; - } + tracing::info!("waiting for tool gate"); + readiness.wait_ready().await; + tracing::info!("tool gate released"); let _guard = if supports_parallel { Either::Left(lock.read().await) } else { diff --git a/codex-rs/utils/readiness/src/lib.rs b/codex-rs/utils/readiness/src/lib.rs index ac3a323fae..872cc10155 100644 --- a/codex-rs/utils/readiness/src/lib.rs +++ b/codex-rs/utils/readiness/src/lib.rs @@ -1,6 +1,7 @@ //! Readiness flag with token-based authorization and async waiting (Tokio). use std::collections::HashSet; +use std::fmt; use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicI32; use std::sync::atomic::Ordering; @@ -79,6 +80,14 @@ impl Default for ReadinessFlag { } } +impl fmt::Debug for ReadinessFlag { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ReadinessFlag") + .field("ready", &self.is_ready()) + .finish() + } +} + #[async_trait::async_trait] impl Readiness for ReadinessFlag { fn is_ready(&self) -> bool { From 4ca1a99a4714c66ffa87d3d341f1ddafaf79f36f Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 24 Oct 2025 11:23:03 +0100 Subject: [PATCH 03/11] V3 --- codex-rs/core/src/codex.rs | 29 +++---- codex-rs/core/src/tasks/ghost_snapshot.rs | 93 +++++++++++++---------- codex-rs/utils/readiness/src/lib.rs | 44 +++++++++-- 3 files changed, 105 insertions(+), 61 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 52843bd51c..9a86e4a30c 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -101,10 +101,12 @@ use crate::state::ActiveTurn; use crate::state::SessionServices; use crate::state::SessionState; use crate::state::TaskKind; -use crate::tasks::{CompactTask, SessionTask, SessionTaskContext}; +use crate::tasks::CompactTask; use crate::tasks::GhostSnapshotTask; use crate::tasks::RegularTask; use crate::tasks::ReviewTask; +use crate::tasks::SessionTask; +use crate::tasks::SessionTaskContext; use crate::tools::ToolRouter; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::parallel::ToolCallRuntime; @@ -408,7 +410,7 @@ impl Session { is_review_mode: false, final_output_json_schema: None, codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), - tool_call_gate: Arc::new(ReadinessFlag::new()) + tool_call_gate: Arc::new(ReadinessFlag::new()), } } @@ -1058,15 +1060,15 @@ impl Session { }; info!("spawning ghost snapshot task"); - let task = GhostSnapshotTask::new( - token, - ); - Arc::new(task).run( - Arc::new(SessionTaskContext::new(self.clone())), - turn_context.clone(), - Vec::new(), - cancellation_token - ).await; + let task = GhostSnapshotTask::new(token); + Arc::new(task) + .run( + Arc::new(SessionTaskContext::new(self.clone())), + turn_context.clone(), + Vec::new(), + cancellation_token, + ) + .await; } /// Returns the input if there was no task running to inject into @@ -1504,7 +1506,7 @@ async fn spawn_review_thread( is_review_mode: true, final_output_json_schema: None, codex_linux_sandbox_exe: parent_turn_context.codex_linux_sandbox_exe.clone(), - tool_call_gate: Arc::new(ReadinessFlag::new()) + tool_call_gate: Arc::new(ReadinessFlag::new()), }; // Seed the child task with the review prompt as the initial user message. @@ -1568,7 +1570,8 @@ pub(crate) async fn run_task( .await; } - sess.maybe_start_ghost_snapshot(Arc::clone(&turn_context), cancellation_token.child_token()).await; + sess.maybe_start_ghost_snapshot(Arc::clone(&turn_context), cancellation_token.child_token()) + .await; let mut last_agent_message: Option = None; // 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. diff --git a/codex-rs/core/src/tasks/ghost_snapshot.rs b/codex-rs/core/src/tasks/ghost_snapshot.rs index 5ce9b4d89a..2e4d84cc85 100644 --- a/codex-rs/core/src/tasks/ghost_snapshot.rs +++ b/codex-rs/core/src/tasks/ghost_snapshot.rs @@ -1,10 +1,8 @@ -use std::borrow::ToOwned; -use std::sync::Arc; -use async_trait::async_trait; use crate::codex::TurnContext; use crate::state::TaskKind; use crate::tasks::SessionTask; use crate::tasks::SessionTaskContext; +use async_trait::async_trait; use codex_git_tooling::CreateGhostCommitOptions; use codex_git_tooling::GitToolingError; use codex_git_tooling::create_ghost_commit; @@ -12,6 +10,8 @@ use codex_protocol::models::ResponseItem; use codex_protocol::user_input::UserInput; use codex_utils_readiness::Readiness; use codex_utils_readiness::Token; +use std::borrow::ToOwned; +use std::sync::Arc; use tokio_util::sync::CancellationToken; use tracing::info; use tracing::warn; @@ -31,42 +31,55 @@ impl SessionTask for GhostSnapshotTask { session: Arc, ctx: Arc, _input: Vec, - cancellation_token: CancellationToken, // TODO handle cancellation token with a tokio select + cancellation_token: CancellationToken, ) -> Option { tokio::task::spawn(async move { - let repo_path = ctx.cwd.clone(); - let options = CreateGhostCommitOptions::new(&repo_path); - let ghost_commit = create_ghost_commit(&options); - info!("ghost snapshot blocking task finished"); - match ghost_commit { - Ok(ghost_commit) => { - session - .session - .record_conversation_items(&[ResponseItem::GhostSnapshot { - commit_id: ghost_commit.id().to_string(), - parent: ghost_commit.parent().map(ToOwned::to_owned), - }]) - .await; - info!("ghost commit captured: {}", ghost_commit.id()); - } - Err(err) => { - warn!( - sub_id = ctx.sub_id.as_str(), - "failed to capture ghost snapshot: {err}" - ); - let message = match err { - GitToolingError::NotAGitRepository { .. } => { - "Snapshots disabled: current directory is not a Git repository." - .to_string() + let token = self.token; + let ctx_for_task = Arc::clone(&ctx); + let cancelled = tokio::select! { + _ = cancellation_token.cancelled() => true, + _ = async { + let repo_path = ctx_for_task.cwd.clone(); + let options = CreateGhostCommitOptions::new(&repo_path); + let ghost_commit = create_ghost_commit(&options); + info!("ghost snapshot blocking task finished"); + match ghost_commit { + Ok(ghost_commit) => { + session + .session + .record_conversation_items(&[ResponseItem::GhostSnapshot { + commit_id: ghost_commit.id().to_string(), + parent: ghost_commit.parent().map(ToOwned::to_owned), + }]) + .await; + info!("ghost commit captured: {}", ghost_commit.id()); + } + Err(err) => { + warn!( + sub_id = ctx_for_task.sub_id.as_str(), + "failed to capture ghost snapshot: {err}" + ); + let message = match err { + GitToolingError::NotAGitRepository { .. } => { + "Snapshots disabled: current directory is not a Git repository." + .to_string() + } + _ => format!("Snapshots disabled after ghost snapshot error: {err}."), + }; + session + .session + .notify_background_event(&ctx_for_task, message) + .await; } - _ => format!("Snapshots disabled after ghost snapshot error: {err}."), - }; - session.session - .notify_background_event(&ctx, message) - .await; - } + } + } => false, + }; + + if cancelled { + info!("ghost snapshot task cancelled"); } - match ctx.tool_call_gate.mark_ready(self.token).await { + + match ctx.tool_call_gate.mark_ready(token).await { Ok(true) => info!("ghost snapshot gate marked ready"), Ok(false) => warn!("ghost snapshot gate already ready"), Err(err) => warn!("failed to mark ghost snapshot ready: {err}"), @@ -77,11 +90,7 @@ impl SessionTask for GhostSnapshotTask { } impl GhostSnapshotTask { - pub(crate) fn new( - token: Token, - ) -> Self { - Self { - token, - } + pub(crate) fn new(token: Token) -> Self { + Self { token } } -} \ No newline at end of file +} diff --git a/codex-rs/utils/readiness/src/lib.rs b/codex-rs/utils/readiness/src/lib.rs index 872cc10155..33de2377d4 100644 --- a/codex-rs/utils/readiness/src/lib.rs +++ b/codex-rs/utils/readiness/src/lib.rs @@ -72,6 +72,10 @@ impl ReadinessFlag { .map_err(|_| errors::ReadinessError::TokenLockFailed)?; Ok(f(&mut guard)) } + + fn load_ready(&self) -> bool { + self.ready.load(Ordering::Acquire) + } } impl Default for ReadinessFlag { @@ -83,7 +87,7 @@ impl Default for ReadinessFlag { impl fmt::Debug for ReadinessFlag { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("ReadinessFlag") - .field("ready", &self.is_ready()) + .field("ready", &self.load_ready()) .finish() } } @@ -91,11 +95,25 @@ impl fmt::Debug for ReadinessFlag { #[async_trait::async_trait] impl Readiness for ReadinessFlag { fn is_ready(&self) -> bool { - self.ready.load(Ordering::Acquire) + if self.load_ready() { + return true; + } + + if let Ok(tokens) = self.tokens.try_lock() + && tokens.is_empty() { + let was_ready = self.ready.swap(true, Ordering::AcqRel); + drop(tokens); + if !was_ready { + let _ = self.tx.send(true); + } + return true; + } + + self.load_ready() } async fn subscribe(&self) -> Result { - if self.is_ready() { + if self.load_ready() { return Err(errors::ReadinessError::FlagAlreadyReady); } @@ -106,7 +124,7 @@ impl Readiness for ReadinessFlag { // check above and inserting the token. let inserted = self .with_tokens(|tokens| { - if self.is_ready() { + if self.load_ready() { return false; } tokens.insert(token); @@ -122,7 +140,7 @@ impl Readiness for ReadinessFlag { } async fn mark_ready(&self, token: Token) -> Result { - if self.is_ready() { + if self.load_ready() { return Ok(false); } if token.0 == 0 { @@ -211,7 +229,8 @@ mod tests { async fn mark_ready_rejects_unknown_token() -> Result<(), ReadinessError> { let flag = ReadinessFlag::new(); assert!(!flag.mark_ready(Token(42)).await?); - assert!(!flag.is_ready()); + assert!(!flag.load_ready()); + assert!(flag.is_ready()); Ok(()) } @@ -242,6 +261,19 @@ mod tests { Ok(()) } + #[tokio::test] + async fn is_ready_without_subscribers_marks_flag_ready() -> Result<(), ReadinessError> { + let flag = ReadinessFlag::new(); + + assert!(flag.is_ready()); + assert!(flag.is_ready()); + assert_matches!( + flag.subscribe().await, + Err(ReadinessError::FlagAlreadyReady) + ); + Ok(()) + } + #[tokio::test] async fn subscribe_returns_error_when_lock_is_held() { let flag = ReadinessFlag::new(); From ad1545a6192535c15953c4af02d7acb3cce8fac9 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 24 Oct 2025 11:28:02 +0100 Subject: [PATCH 04/11] FMT --- codex-rs/utils/readiness/src/lib.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/codex-rs/utils/readiness/src/lib.rs b/codex-rs/utils/readiness/src/lib.rs index 33de2377d4..8b92f9387a 100644 --- a/codex-rs/utils/readiness/src/lib.rs +++ b/codex-rs/utils/readiness/src/lib.rs @@ -100,14 +100,15 @@ impl Readiness for ReadinessFlag { } if let Ok(tokens) = self.tokens.try_lock() - && tokens.is_empty() { - let was_ready = self.ready.swap(true, Ordering::AcqRel); - drop(tokens); - if !was_ready { - let _ = self.tx.send(true); - } - return true; + && tokens.is_empty() + { + let was_ready = self.ready.swap(true, Ordering::AcqRel); + drop(tokens); + if !was_ready { + let _ = self.tx.send(true); } + return true; + } self.load_ready() } From 307fcac0114d4030c175efad63b4ddd64c4c83b0 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 24 Oct 2025 11:49:21 +0100 Subject: [PATCH 05/11] NIT --- codex-rs/core/src/codex.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 2a4d9d3858..2ad86fdb1a 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2046,9 +2046,6 @@ async fn try_run_turn( sess.send_event(&turn_context, msg).await; } - sess.update_token_usage_info(turn_context.as_ref(), token_usage.as_ref()) - .await; - let result = TurnRunResult { processed_items, total_token_usage: token_usage.clone(), From 67cecb7edf08ad1901bfe1bfa9c37005f3a23f1e Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 24 Oct 2025 12:10:30 +0100 Subject: [PATCH 06/11] Make it cleaner --- codex-rs/core/src/codex.rs | 18 ++++++++++++++- codex-rs/core/src/features.rs | 8 +++++++ codex-rs/core/src/tasks/ghost_snapshot.rs | 28 ++++++++++++++++++----- 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 2ad86fdb1a..c561b5cecb 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -183,6 +183,7 @@ impl Codex { sandbox_policy: config.sandbox_policy.clone(), cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), + features: config.features.clone(), }; // Generate a unique ID for the lifetime of this Codex session. @@ -318,6 +319,9 @@ pub(crate) struct SessionConfiguration { /// operate deterministically. cwd: PathBuf, + /// Set of feature flags for this session + features: Features, + // TODO(pakrym): Remove config from here original_config_do_not_use: Arc, } @@ -1049,7 +1053,15 @@ impl Session { turn_context: Arc, cancellation_token: CancellationToken, ) { - if turn_context.is_review_mode { + if turn_context.is_review_mode + || !self + .state + .lock() + .await + .session_configuration + .features + .enabled(Feature::GhostCommit) + { return; } @@ -2280,6 +2292,8 @@ fn is_mcp_client_startup_timeout_error(error: &anyhow::Error) -> bool { || error_message.contains("timed out handshaking with MCP server") } +use crate::features::Feature; +use crate::features::Features; #[cfg(test)] pub(crate) use tests::make_session_and_context; @@ -2596,6 +2610,7 @@ mod tests { sandbox_policy: config.sandbox_policy.clone(), cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), + features: Features::default(), }; let state = SessionState::new(session_configuration.clone()); @@ -2664,6 +2679,7 @@ mod tests { sandbox_policy: config.sandbox_policy.clone(), cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), + features: Features::default(), }; let state = SessionState::new(session_configuration.clone()); diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index ead4604d54..a871e81e23 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -39,6 +39,8 @@ pub enum Feature { ViewImageTool, /// Allow the model to request web searches. WebSearchRequest, + /// Create a ghost commit at each turn. + GhostCommit, } impl Feature { @@ -236,4 +238,10 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Stable, default_enabled: false, }, + FeatureSpec { + id: Feature::GhostCommit, + key: "ghost_commit", + stage: Stage::Experimental, + default_enabled: false, + }, ]; diff --git a/codex-rs/core/src/tasks/ghost_snapshot.rs b/codex-rs/core/src/tasks/ghost_snapshot.rs index 2e4d84cc85..97687a6fa5 100644 --- a/codex-rs/core/src/tasks/ghost_snapshot.rs +++ b/codex-rs/core/src/tasks/ghost_snapshot.rs @@ -40,11 +40,15 @@ impl SessionTask for GhostSnapshotTask { _ = cancellation_token.cancelled() => true, _ = async { let repo_path = ctx_for_task.cwd.clone(); - let options = CreateGhostCommitOptions::new(&repo_path); - let ghost_commit = create_ghost_commit(&options); - info!("ghost snapshot blocking task finished"); - match ghost_commit { - Ok(ghost_commit) => { + // Required to run in a dedicated blocking pool. + match tokio::task::spawn_blocking(move || { + let options = CreateGhostCommitOptions::new(&repo_path); + create_ghost_commit(&options) + }) + .await + { + Ok(Ok(ghost_commit)) => { + info!("ghost snapshot blocking task finished"); session .session .record_conversation_items(&[ResponseItem::GhostSnapshot { @@ -54,7 +58,7 @@ impl SessionTask for GhostSnapshotTask { .await; info!("ghost commit captured: {}", ghost_commit.id()); } - Err(err) => { + Ok(Err(err)) => { warn!( sub_id = ctx_for_task.sub_id.as_str(), "failed to capture ghost snapshot: {err}" @@ -71,6 +75,18 @@ impl SessionTask for GhostSnapshotTask { .notify_background_event(&ctx_for_task, message) .await; } + Err(err) => { + warn!( + sub_id = ctx_for_task.sub_id.as_str(), + "ghost snapshot task panicked: {err}" + ); + let message = + format!("Snapshots disabled after ghost snapshot panic: {err}."); + session + .session + .notify_background_event(&ctx_for_task, message) + .await; + } } } => false, }; From 40d30400d8f66624c24f11ba36158dca9c0716b9 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 24 Oct 2025 13:26:09 +0100 Subject: [PATCH 07/11] V1 --- codex-rs/Cargo.lock | 1 - codex-rs/core/src/rollout/policy.rs | 2 + codex-rs/protocol/src/protocol.rs | 20 ++++++ codex-rs/tui/Cargo.toml | 1 - codex-rs/tui/src/chatwidget.rs | 96 ++++++++-------------------- codex-rs/tui/src/chatwidget/tests.rs | 84 +++++++++++++++++++++++- codex-rs/tui/src/slash_command.rs | 2 +- 7 files changed, 133 insertions(+), 73 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 979b67ee4b..a5307efd45 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1428,7 +1428,6 @@ dependencies = [ "codex-core", "codex-feedback", "codex-file-search", - "codex-git-tooling", "codex-login", "codex-ollama", "codex-protocol", diff --git a/codex-rs/core/src/rollout/policy.rs b/codex-rs/core/src/rollout/policy.rs index 357d3d5adb..ba275aaa6d 100644 --- a/codex-rs/core/src/rollout/policy.rs +++ b/codex-rs/core/src/rollout/policy.rs @@ -43,6 +43,7 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::TokenCount(_) | EventMsg::EnteredReviewMode(_) | EventMsg::ExitedReviewMode(_) + | EventMsg::UndoCompleted(_) | EventMsg::TurnAborted(_) => true, EventMsg::Error(_) | EventMsg::TaskStarted(_) @@ -67,6 +68,7 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::PatchApplyEnd(_) | EventMsg::TurnDiff(_) | EventMsg::GetHistoryEntryResponse(_) + | EventMsg::UndoStarted(_) | EventMsg::McpListToolsResponse(_) | EventMsg::ListCustomPromptsResponse(_) | EventMsg::PlanUpdate(_) diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 105f028049..e3b971bede 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -176,6 +176,9 @@ pub enum Op { /// to generate a summary which will be returned as an AgentMessage event. Compact, + /// Request Codex to undo a turn (turn are stacked so it is the same effect as CMD + Z). + Undo, + /// Request a code review from the agent. Review { review_request: ReviewRequest }, @@ -484,6 +487,10 @@ pub enum EventMsg { BackgroundEvent(BackgroundEventEvent), + UndoStarted(UndoStartedEvent), + + UndoCompleted(UndoCompletedEvent), + /// Notification that a model stream experienced an error or disconnect /// and the system is handling it (e.g., retrying with backoff). StreamError(StreamErrorEvent), @@ -1158,6 +1165,19 @@ pub struct BackgroundEventEvent { pub message: String, } +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +pub struct UndoStartedEvent { + #[serde(skip_serializing_if = "Option::is_none")] + pub message: Option, +} + +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +pub struct UndoCompletedEvent { + pub success: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub message: Option, +} + #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct StreamErrorEvent { pub message: String, diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 824e4fee31..f087202c22 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -35,7 +35,6 @@ codex-common = { workspace = true, features = [ ] } codex-core = { workspace = true } codex-file-search = { workspace = true } -codex-git-tooling = { workspace = true } codex-login = { workspace = true } codex-ollama = { workspace = true } codex-protocol = { workspace = true } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index c090d212da..a2a94eb065 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -37,6 +37,8 @@ use codex_core::protocol::TokenUsage; use codex_core::protocol::TokenUsageInfo; use codex_core::protocol::TurnAbortReason; use codex_core::protocol::TurnDiffEvent; +use codex_core::protocol::UndoCompletedEvent; +use codex_core::protocol::UndoStartedEvent; use codex_core::protocol::UserMessageEvent; use codex_core::protocol::ViewImageToolCallEvent; use codex_core::protocol::WebSearchBeginEvent; @@ -113,16 +115,9 @@ use codex_core::protocol::AskForApproval; use codex_core::protocol::SandboxPolicy; use codex_core::protocol_config_types::ReasoningEffort as ReasoningEffortConfig; use codex_file_search::FileMatch; -use codex_git_tooling::CreateGhostCommitOptions; -use codex_git_tooling::GhostCommit; -use codex_git_tooling::GitToolingError; -use codex_git_tooling::create_ghost_commit; -use codex_git_tooling::restore_ghost_commit; use codex_protocol::plan_tool::UpdatePlanArgs; use strum::IntoEnumIterator; -const MAX_TRACKED_GHOST_COMMITS: usize = 20; - // Track information about an in-flight exec command. struct RunningCommand { command: Vec, @@ -267,9 +262,6 @@ pub(crate) struct ChatWidget { pending_notification: Option, // Simple review mode flag; used to adjust layout and banners. is_review_mode: bool, - // List of ghost commits corresponding to each turn. - ghost_snapshots: Vec, - ghost_snapshots_disabled: bool, // Whether to add a final message separator after the last message needs_final_message_separator: bool, @@ -636,6 +628,29 @@ impl ChatWidget { debug!("BackgroundEvent: {message}"); } + fn on_undo_started(&mut self, event: UndoStartedEvent) { + let message = event + .message + .unwrap_or_else(|| "Undo in progress...".to_string()); + self.add_info_message(message, None); + } + + fn on_undo_completed(&mut self, event: UndoCompletedEvent) { + let UndoCompletedEvent { success, message } = event; + let message = message.unwrap_or_else(|| { + if success { + "Undo completed successfully.".to_string() + } else { + "Undo failed.".to_string() + } + }); + if success { + self.add_info_message(message, None); + } else { + self.add_error_message(message); + } + } + fn on_stream_error(&mut self, message: String) { if self.retry_status_header.is_none() { self.retry_status_header = Some(self.current_status_header.clone()); @@ -952,8 +967,6 @@ impl ChatWidget { suppress_session_configured_redraw: false, pending_notification: None, is_review_mode: false, - ghost_snapshots: Vec::new(), - ghost_snapshots_disabled: true, needs_final_message_separator: false, last_rendered_width: std::cell::Cell::new(None), feedback, @@ -1019,8 +1032,6 @@ impl ChatWidget { suppress_session_configured_redraw: true, pending_notification: None, is_review_mode: false, - ghost_snapshots: Vec::new(), - ghost_snapshots_disabled: true, needs_final_message_separator: false, last_rendered_width: std::cell::Cell::new(None), feedback, @@ -1184,7 +1195,7 @@ impl ChatWidget { self.app_event_tx.send(AppEvent::ExitRequest); } SlashCommand::Undo => { - self.undo_last_snapshot(); + self.app_event_tx.send(AppEvent::CodexOp(Op::Undo)); } SlashCommand::Diff => { self.add_diff_in_progress(); @@ -1301,8 +1312,6 @@ impl ChatWidget { return; } - self.capture_ghost_snapshot(); - let mut items: Vec = Vec::new(); if !text.is_empty() { @@ -1335,57 +1344,6 @@ impl ChatWidget { self.needs_final_message_separator = false; } - fn capture_ghost_snapshot(&mut self) { - if self.ghost_snapshots_disabled { - return; - } - - let options = CreateGhostCommitOptions::new(&self.config.cwd); - match create_ghost_commit(&options) { - Ok(commit) => { - self.ghost_snapshots.push(commit); - if self.ghost_snapshots.len() > MAX_TRACKED_GHOST_COMMITS { - self.ghost_snapshots.remove(0); - } - } - Err(err) => { - self.ghost_snapshots_disabled = true; - let (message, hint) = match &err { - GitToolingError::NotAGitRepository { .. } => ( - "Snapshots disabled: current directory is not a Git repository." - .to_string(), - None, - ), - _ => ( - format!("Snapshots disabled after error: {err}"), - Some( - "Restart Codex after resolving the issue to re-enable snapshots." - .to_string(), - ), - ), - }; - self.add_info_message(message, hint); - tracing::warn!("failed to create ghost snapshot: {err}"); - } - } - } - - fn undo_last_snapshot(&mut self) { - let Some(commit) = self.ghost_snapshots.pop() else { - self.add_info_message("No snapshot available to undo.".to_string(), None); - return; - }; - - if let Err(err) = restore_ghost_commit(&self.config.cwd, &commit) { - self.add_error_message(format!("Failed to restore snapshot: {err}")); - self.ghost_snapshots.push(commit); - return; - } - - let short_id: String = commit.id().chars().take(8).collect(); - self.add_info_message(format!("Restored workspace to snapshot {short_id}"), None); - } - /// Replay a subset of initial events into the UI to seed the transcript when /// resuming an existing session. This approximates the live event flow and /// is intentionally conservative: only safe-to-replay items are rendered to @@ -1483,6 +1441,8 @@ impl ChatWidget { EventMsg::BackgroundEvent(BackgroundEventEvent { message }) => { self.on_background_event(message) } + EventMsg::UndoStarted(ev) => self.on_undo_started(ev), + EventMsg::UndoCompleted(ev) => self.on_undo_completed(ev), EventMsg::StreamError(StreamErrorEvent { message }) => self.on_stream_error(message), EventMsg::UserMessage(ev) => { if from_replay { diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 738fc31e42..0ff7b1e81c 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -34,6 +34,8 @@ use codex_core::protocol::ReviewRequest; use codex_core::protocol::StreamErrorEvent; use codex_core::protocol::TaskCompleteEvent; use codex_core::protocol::TaskStartedEvent; +use codex_core::protocol::UndoCompletedEvent; +use codex_core::protocol::UndoStartedEvent; use codex_core::protocol::ViewImageToolCallEvent; use codex_protocol::ConversationId; use codex_protocol::plan_tool::PlanItemArg; @@ -294,8 +296,6 @@ fn make_chatwidget_manual() -> ( suppress_session_configured_redraw: false, pending_notification: None, is_review_mode: false, - ghost_snapshots: Vec::new(), - ghost_snapshots_disabled: false, needs_final_message_separator: false, last_rendered_width: std::cell::Cell::new(None), feedback: codex_feedback::CodexFeedback::new(), @@ -845,6 +845,86 @@ fn slash_init_skips_when_project_doc_exists() { ); } +#[test] +fn slash_undo_sends_op() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + chat.dispatch_command(SlashCommand::Undo); + + match rx.try_recv() { + Ok(AppEvent::CodexOp(Op::Undo)) => {} + other => panic!("expected AppEvent::CodexOp(Op::Undo), got {other:?}"), + } +} + +#[test] +fn undo_success_events_render_info_messages() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + chat.handle_codex_event(Event { + id: "turn-1".to_string(), + msg: EventMsg::UndoStarted(UndoStartedEvent { + message: Some("Undo requested for the last turn...".to_string()), + }), + }); + + chat.handle_codex_event(Event { + id: "turn-1".to_string(), + msg: EventMsg::UndoCompleted(UndoCompletedEvent { + success: true, + message: None, + }), + }); + + let cells = drain_insert_history(&mut rx); + assert_eq!(cells.len(), 2, "expected two history entries"); + + let started = lines_to_single_string(&cells[0]); + assert!( + started.contains("Undo requested for the last turn..."), + "expected custom started message, got {started:?}" + ); + + let completed = lines_to_single_string(&cells[1]); + assert!( + completed.contains("Undo completed successfully."), + "expected default success message, got {completed:?}" + ); +} + +#[test] +fn undo_failure_events_render_error_message() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + chat.handle_codex_event(Event { + id: "turn-2".to_string(), + msg: EventMsg::UndoStarted(UndoStartedEvent { message: None }), + }); + + chat.handle_codex_event(Event { + id: "turn-2".to_string(), + msg: EventMsg::UndoCompleted(UndoCompletedEvent { + success: false, + message: Some("Failed to restore workspace state.".to_string()), + }), + }); + + let cells = drain_insert_history(&mut rx); + assert_eq!(cells.len(), 2, "expected two history entries"); + + let started = lines_to_single_string(&cells[0]); + assert!( + started.contains("Undo in progress..."), + "expected default started message, got {started:?}" + ); + + let completed = lines_to_single_string(&cells[1]); + assert!( + completed.contains("Failed to restore workspace state."), + "expected failure message, got {completed:?}" + ); +} + /// The commit picker shows only commit subjects (no timestamps). #[test] fn review_commit_picker_shows_subjects_without_timestamps() { diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 22da09f358..2d293549f1 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -39,7 +39,7 @@ impl SlashCommand { SlashCommand::Init => "create an AGENTS.md file with instructions for Codex", SlashCommand::Compact => "summarize conversation to prevent hitting the context limit", SlashCommand::Review => "review my current changes and find issues", - SlashCommand::Undo => "restore the workspace to the last Codex snapshot", + SlashCommand::Undo => "ask Codex to undo a turn", SlashCommand::Quit => "exit Codex", SlashCommand::Diff => "show git diff (including untracked files)", SlashCommand::Mention => "mention a file", From 4c152e35c75c6ed5b20d3c2a9b9360db9737fbbd Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 24 Oct 2025 13:34:39 +0100 Subject: [PATCH 08/11] V2 --- codex-rs/tui/src/bottom_pane/mod.rs | 15 +++++++++++++++ codex-rs/tui/src/chatwidget.rs | 4 +++- codex-rs/tui/src/chatwidget/tests.rs | 28 ++++++++++++++++------------ 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 69405cd823..6f1b7d0552 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -313,6 +313,11 @@ impl BottomPane { self.ctrl_c_quit_hint } + #[cfg(test)] + pub(crate) fn status_indicator_visible(&self) -> bool { + self.status.is_some() + } + pub(crate) fn show_esc_backtrack_hint(&mut self) { self.esc_backtrack_hint = true; self.composer.set_esc_backtrack_hint(true); @@ -357,6 +362,16 @@ impl BottomPane { } } + pub(crate) fn ensure_status_indicator(&mut self) { + if self.status.is_none() { + self.status = Some(StatusIndicatorWidget::new( + self.app_event_tx.clone(), + self.frame_requester.clone(), + )); + self.request_redraw(); + } + } + pub(crate) fn set_context_window_percent(&mut self, percent: Option) { if self.context_window_percent == percent { return; diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index a2a94eb065..658af03364 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -629,14 +629,16 @@ impl ChatWidget { } fn on_undo_started(&mut self, event: UndoStartedEvent) { + self.bottom_pane.ensure_status_indicator(); let message = event .message .unwrap_or_else(|| "Undo in progress...".to_string()); - self.add_info_message(message, None); + self.set_status_header(message.clone()); } fn on_undo_completed(&mut self, event: UndoCompletedEvent) { let UndoCompletedEvent { success, message } = event; + self.bottom_pane.hide_status_indicator(); let message = message.unwrap_or_else(|| { if success { "Undo completed successfully.".to_string() diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 0ff7b1e81c..ab2f4a9ef4 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -867,6 +867,10 @@ fn undo_success_events_render_info_messages() { message: Some("Undo requested for the last turn...".to_string()), }), }); + assert!( + chat.bottom_pane.status_indicator_visible(), + "status indicator should be visible during undo" + ); chat.handle_codex_event(Event { id: "turn-1".to_string(), @@ -877,15 +881,13 @@ fn undo_success_events_render_info_messages() { }); let cells = drain_insert_history(&mut rx); - assert_eq!(cells.len(), 2, "expected two history entries"); - - let started = lines_to_single_string(&cells[0]); + assert_eq!(cells.len(), 1, "expected final status only"); assert!( - started.contains("Undo requested for the last turn..."), - "expected custom started message, got {started:?}" + !chat.bottom_pane.status_indicator_visible(), + "status indicator should be hidden after successful undo" ); - let completed = lines_to_single_string(&cells[1]); + let completed = lines_to_single_string(&cells[0]); assert!( completed.contains("Undo completed successfully."), "expected default success message, got {completed:?}" @@ -900,6 +902,10 @@ fn undo_failure_events_render_error_message() { id: "turn-2".to_string(), msg: EventMsg::UndoStarted(UndoStartedEvent { message: None }), }); + assert!( + chat.bottom_pane.status_indicator_visible(), + "status indicator should be visible during undo" + ); chat.handle_codex_event(Event { id: "turn-2".to_string(), @@ -910,15 +916,13 @@ fn undo_failure_events_render_error_message() { }); let cells = drain_insert_history(&mut rx); - assert_eq!(cells.len(), 2, "expected two history entries"); - - let started = lines_to_single_string(&cells[0]); + assert_eq!(cells.len(), 1, "expected final status only"); assert!( - started.contains("Undo in progress..."), - "expected default started message, got {started:?}" + !chat.bottom_pane.status_indicator_visible(), + "status indicator should be hidden after failed undo" ); - let completed = lines_to_single_string(&cells[1]); + let completed = lines_to_single_string(&cells[0]); assert!( completed.contains("Failed to restore workspace state."), "expected failure message, got {completed:?}" From c396a1072dd94ccc4b4aa65ab8aa3cc7fcdcd87e Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 24 Oct 2025 13:40:34 +0100 Subject: [PATCH 09/11] V3 --- codex-rs/tui/src/chatwidget.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 658af03364..d36c514298 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -633,7 +633,7 @@ impl ChatWidget { let message = event .message .unwrap_or_else(|| "Undo in progress...".to_string()); - self.set_status_header(message.clone()); + self.set_status_header(message); } fn on_undo_completed(&mut self, event: UndoCompletedEvent) { From 129912ce2d40a4cf766bd8d8eefdf380eac6c83b Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 27 Oct 2025 10:04:54 +0000 Subject: [PATCH 10/11] feat: undo wiring (#5630) --- codex-rs/Cargo.lock | 16 +- codex-rs/core/src/codex.rs | 10 +- codex-rs/core/src/tasks/ghost_snapshot.rs | 4 +- codex-rs/core/src/tasks/mod.rs | 2 + codex-rs/core/src/tasks/undo.rs | 117 +++++++++ .../src/event_processor_with_human_output.rs | 45 ++-- codex-rs/git-tooling/Cargo.toml | 15 +- codex-rs/git-tooling/src/ghost_commits.rs | 226 +++++++++++++++++- codex-rs/git-tooling/src/lib.rs | 39 ++- codex-rs/git-tooling/src/operations.rs | 21 ++ codex-rs/mcp-server/src/codex_tool_runner.rs | 2 + codex-rs/protocol/Cargo.toml | 2 + codex-rs/protocol/src/models.rs | 5 +- codex-rs/tui/src/slash_command.rs | 11 +- 14 files changed, 450 insertions(+), 65 deletions(-) create mode 100644 codex-rs/core/src/tasks/undo.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index a5307efd45..679e445be2 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1209,8 +1209,11 @@ version = "0.0.0" dependencies = [ "assert_matches", "pretty_assertions", + "schemars 0.8.22", + "serde", "tempfile", "thiserror 2.0.16", + "ts-rs", "walkdir", ] @@ -1328,6 +1331,7 @@ version = "0.0.0" dependencies = [ "anyhow", "base64", + "codex-git-tooling", "icu_decimal", "icu_locale_core", "mcp-types", @@ -5457,9 +5461,9 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.226" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0dca6411025b24b60bfa7ec1fe1f8e710ac09782dca409ee8237ba74b51295fd" +checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e" dependencies = [ "serde_core", "serde_derive", @@ -5467,18 +5471,18 @@ dependencies = [ [[package]] name = "serde_core" -version = "1.0.226" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba2ba63999edb9dac981fb34b3e5c0d111a69b0924e253ed29d83f7c99e966a4" +checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.226" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8db53ae22f34573731bafa1db20f04027b2d25e02d8205921b569171699cdb33" +checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index c561b5cecb..6ef67dcb26 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -109,6 +109,7 @@ use crate::tasks::RegularTask; use crate::tasks::ReviewTask; use crate::tasks::SessionTask; use crate::tasks::SessionTaskContext; +use crate::tasks::UndoTask; use crate::tools::ToolRouter; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::parallel::ToolCallRuntime; @@ -906,7 +907,7 @@ impl Session { state.record_items(items.iter()); } - async fn replace_history(&self, items: Vec) { + pub(crate) async fn replace_history(&self, items: Vec) { let mut state = self.state.lock().await; state.replace_history(items); } @@ -1358,6 +1359,13 @@ async fn submission_loop(sess: Arc, config: Arc, rx_sub: Receiv }; sess.send_event_raw(event).await; } + Op::Undo => { + let turn_context = sess + .new_turn_with_sub_id(sub.id.clone(), SessionSettingsUpdate::default()) + .await; + sess.spawn_task(turn_context, Vec::new(), UndoTask::new()) + .await; + } Op::Compact => { let turn_context = sess .new_turn_with_sub_id(sub.id.clone(), SessionSettingsUpdate::default()) diff --git a/codex-rs/core/src/tasks/ghost_snapshot.rs b/codex-rs/core/src/tasks/ghost_snapshot.rs index 97687a6fa5..8dc6a87e2f 100644 --- a/codex-rs/core/src/tasks/ghost_snapshot.rs +++ b/codex-rs/core/src/tasks/ghost_snapshot.rs @@ -10,7 +10,6 @@ use codex_protocol::models::ResponseItem; use codex_protocol::user_input::UserInput; use codex_utils_readiness::Readiness; use codex_utils_readiness::Token; -use std::borrow::ToOwned; use std::sync::Arc; use tokio_util::sync::CancellationToken; use tracing::info; @@ -52,8 +51,7 @@ impl SessionTask for GhostSnapshotTask { session .session .record_conversation_items(&[ResponseItem::GhostSnapshot { - commit_id: ghost_commit.id().to_string(), - parent: ghost_commit.parent().map(ToOwned::to_owned), + ghost_commit: ghost_commit.clone(), }]) .await; info!("ghost commit captured: {}", ghost_commit.id()); diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index a5afbca2e3..466bfe21a9 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -2,6 +2,7 @@ mod compact; mod ghost_snapshot; mod regular; mod review; +mod undo; use std::sync::Arc; use std::time::Duration; @@ -29,6 +30,7 @@ pub(crate) use compact::CompactTask; pub(crate) use ghost_snapshot::GhostSnapshotTask; pub(crate) use regular::RegularTask; pub(crate) use review::ReviewTask; +pub(crate) use undo::UndoTask; const GRACEFULL_INTERRUPTION_TIMEOUT_MS: u64 = 100; diff --git a/codex-rs/core/src/tasks/undo.rs b/codex-rs/core/src/tasks/undo.rs new file mode 100644 index 0000000000..e834eea02d --- /dev/null +++ b/codex-rs/core/src/tasks/undo.rs @@ -0,0 +1,117 @@ +use std::sync::Arc; + +use crate::codex::TurnContext; +use crate::protocol::EventMsg; +use crate::protocol::UndoCompletedEvent; +use crate::protocol::UndoStartedEvent; +use crate::state::TaskKind; +use crate::tasks::SessionTask; +use crate::tasks::SessionTaskContext; +use async_trait::async_trait; +use codex_git_tooling::restore_ghost_commit; +use codex_protocol::models::ResponseItem; +use codex_protocol::user_input::UserInput; +use tokio_util::sync::CancellationToken; +use tracing::error; +use tracing::info; +use tracing::warn; + +pub(crate) struct UndoTask; + +impl UndoTask { + pub(crate) fn new() -> Self { + Self + } +} + +#[async_trait] +impl SessionTask for UndoTask { + fn kind(&self) -> TaskKind { + TaskKind::Regular + } + + async fn run( + self: Arc, + session: Arc, + ctx: Arc, + _input: Vec, + cancellation_token: CancellationToken, + ) -> Option { + let sess = session.clone_session(); + sess.send_event( + ctx.as_ref(), + EventMsg::UndoStarted(UndoStartedEvent { + message: Some("Undo in progress...".to_string()), + }), + ) + .await; + + if cancellation_token.is_cancelled() { + sess.send_event( + ctx.as_ref(), + EventMsg::UndoCompleted(UndoCompletedEvent { + success: false, + message: Some("Undo cancelled.".to_string()), + }), + ) + .await; + return None; + } + + let mut history = sess.clone_history().await; + let mut items = history.get_history(); + let mut completed = UndoCompletedEvent { + success: false, + message: None, + }; + + let Some((idx, ghost_commit)) = + items + .iter() + .enumerate() + .rev() + .find_map(|(idx, item)| match item { + ResponseItem::GhostSnapshot { ghost_commit } => { + Some((idx, ghost_commit.clone())) + } + _ => None, + }) + else { + completed.message = Some("No ghost snapshot available to undo.".to_string()); + sess.send_event(ctx.as_ref(), EventMsg::UndoCompleted(completed)) + .await; + return None; + }; + + let commit_id = ghost_commit.id().to_string(); + let repo_path = ctx.cwd.clone(); + let restore_result = + tokio::task::spawn_blocking(move || restore_ghost_commit(&repo_path, &ghost_commit)) + .await; + + match restore_result { + Ok(Ok(())) => { + items.remove(idx); + sess.replace_history(items).await; + let short_id: String = commit_id.chars().take(7).collect(); + info!(commit_id = commit_id, "Undo restored ghost snapshot"); + completed.success = true; + completed.message = Some(format!("Undo restored snapshot {short_id}.")); + } + Ok(Err(err)) => { + let message = format!("Failed to restore snapshot {commit_id}: {err}"); + warn!("{message}"); + completed.message = Some(message); + } + Err(err) => { + let message = format!("Failed to restore snapshot {commit_id}: {err}"); + error!("{message}"); + completed.message = Some(message); + } + } + + sess.send_event(ctx.as_ref(), EventMsg::UndoCompleted(completed)) + .await; + None + } +} diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 9a85c5de71..f0fda4ab34 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -20,7 +20,6 @@ use codex_core::protocol::StreamErrorEvent; use codex_core::protocol::TaskCompleteEvent; use codex_core::protocol::TurnAbortReason; use codex_core::protocol::TurnDiffEvent; -use codex_core::protocol::WebSearchBeginEvent; use codex_core::protocol::WebSearchEndEvent; use codex_protocol::num_format::format_with_separators; use owo_colors::OwoColorize; @@ -216,7 +215,6 @@ impl EventProcessor for EventProcessorWithHumanOutput { cwd.to_string_lossy(), ); } - EventMsg::ExecCommandOutputDelta(_) => {} EventMsg::ExecCommandEnd(ExecCommandEndEvent { aggregated_output, duration, @@ -283,7 +281,6 @@ impl EventProcessor for EventProcessorWithHumanOutput { } } } - EventMsg::WebSearchBegin(WebSearchBeginEvent { call_id: _ }) => {} EventMsg::WebSearchEnd(WebSearchEndEvent { call_id: _, query }) => { ts_msg!(self, "🌐 Searched: {query}"); } @@ -411,12 +408,6 @@ impl EventProcessor for EventProcessorWithHumanOutput { ); eprintln!("{unified_diff}"); } - EventMsg::ExecApprovalRequest(_) => { - // Should we exit? - } - EventMsg::ApplyPatchApprovalRequest(_) => { - // Should we exit? - } EventMsg::AgentReasoning(agent_reasoning_event) => { if self.show_agent_reasoning { ts_msg!( @@ -481,15 +472,6 @@ impl EventProcessor for EventProcessorWithHumanOutput { } } } - EventMsg::GetHistoryEntryResponse(_) => { - // Currently ignored in exec output. - } - EventMsg::McpListToolsResponse(_) => { - // Currently ignored in exec output. - } - EventMsg::ListCustomPromptsResponse(_) => { - // Currently ignored in exec output. - } EventMsg::ViewImageToolCall(view) => { ts_msg!( self, @@ -510,15 +492,24 @@ impl EventProcessor for EventProcessorWithHumanOutput { } }, EventMsg::ShutdownComplete => return CodexStatus::Shutdown, - EventMsg::ConversationPath(_) => {} - EventMsg::UserMessage(_) => {} - EventMsg::EnteredReviewMode(_) => {} - EventMsg::ExitedReviewMode(_) => {} - EventMsg::AgentMessageDelta(_) => {} - EventMsg::AgentReasoningDelta(_) => {} - EventMsg::AgentReasoningRawContentDelta(_) => {} - EventMsg::ItemStarted(_) => {} - EventMsg::ItemCompleted(_) => {} + EventMsg::WebSearchBegin(_) + | EventMsg::ExecApprovalRequest(_) + | EventMsg::ApplyPatchApprovalRequest(_) + | EventMsg::ExecCommandOutputDelta(_) + | EventMsg::GetHistoryEntryResponse(_) + | EventMsg::McpListToolsResponse(_) + | EventMsg::ListCustomPromptsResponse(_) + | EventMsg::ConversationPath(_) + | EventMsg::UserMessage(_) + | EventMsg::EnteredReviewMode(_) + | EventMsg::ExitedReviewMode(_) + | EventMsg::AgentMessageDelta(_) + | EventMsg::AgentReasoningDelta(_) + | EventMsg::AgentReasoningRawContentDelta(_) + | EventMsg::ItemStarted(_) + | EventMsg::ItemCompleted(_) + | EventMsg::UndoCompleted(_) + | EventMsg::UndoStarted(_) => {} } CodexStatus::Running } diff --git a/codex-rs/git-tooling/Cargo.toml b/codex-rs/git-tooling/Cargo.toml index 183221f3e1..c30d58de8d 100644 --- a/codex-rs/git-tooling/Cargo.toml +++ b/codex-rs/git-tooling/Cargo.toml @@ -9,13 +9,20 @@ name = "codex_git_tooling" path = "src/lib.rs" [dependencies] -tempfile = "3" -thiserror = "2" -walkdir = "2" +tempfile = { workspace = true } +thiserror = { workspace = true } +walkdir = { workspace = true } +schemars = { workspace = true } +serde = { workspace = true, features = ["derive"] } +ts-rs = { workspace = true, features = [ + "uuid-impl", + "serde-json-impl", + "no-serde-warnings", +] } [lints] workspace = true [dev-dependencies] assert_matches = { workspace = true } -pretty_assertions = "1.4.1" +pretty_assertions = { workspace = true } diff --git a/codex-rs/git-tooling/src/ghost_commits.rs b/codex-rs/git-tooling/src/ghost_commits.rs index 6c2a86166c..c5ebd7c02c 100644 --- a/codex-rs/git-tooling/src/ghost_commits.rs +++ b/codex-rs/git-tooling/src/ghost_commits.rs @@ -1,4 +1,7 @@ +use std::collections::HashSet; use std::ffi::OsString; +use std::fs; +use std::io; use std::path::Path; use std::path::PathBuf; @@ -14,6 +17,7 @@ use crate::operations::resolve_head; use crate::operations::resolve_repository_root; use crate::operations::run_git_for_status; use crate::operations::run_git_for_stdout; +use crate::operations::run_git_for_stdout_all; /// Default commit message used for ghost commits when none is provided. const DEFAULT_COMMIT_MESSAGE: &str = "codex snapshot"; @@ -69,6 +73,8 @@ pub fn create_ghost_commit( let repo_root = resolve_repository_root(options.repo_path)?; let repo_prefix = repo_subdir(repo_root.as_path(), options.repo_path); let parent = resolve_head(repo_root.as_path())?; + let existing_untracked = + capture_existing_untracked(repo_root.as_path(), repo_prefix.as_deref())?; let normalized_force = options .force_include @@ -84,6 +90,16 @@ pub fn create_ghost_commit( OsString::from(index_path.as_os_str()), )]; + // Pre-populate the temporary index with HEAD so unchanged tracked files + // are included in the snapshot tree. + if let Some(parent_sha) = parent.as_deref() { + run_git_for_status( + repo_root.as_path(), + vec![OsString::from("read-tree"), OsString::from(parent_sha)], + Some(base_env.as_slice()), + )?; + } + let mut add_args = vec![OsString::from("add"), OsString::from("--all")]; if let Some(prefix) = repo_prefix.as_deref() { add_args.extend([OsString::from("--"), prefix.as_os_str().to_os_string()]); @@ -127,12 +143,29 @@ pub fn create_ghost_commit( Some(commit_env.as_slice()), )?; - Ok(GhostCommit::new(commit_id, parent)) + Ok(GhostCommit::new( + commit_id, + parent, + existing_untracked.files, + existing_untracked.dirs, + )) } /// Restore the working tree to match the provided ghost commit. pub fn restore_ghost_commit(repo_path: &Path, commit: &GhostCommit) -> Result<(), GitToolingError> { - restore_to_commit(repo_path, commit.id()) + ensure_git_repository(repo_path)?; + + let repo_root = resolve_repository_root(repo_path)?; + let repo_prefix = repo_subdir(repo_root.as_path(), repo_path); + let current_untracked = + capture_existing_untracked(repo_root.as_path(), repo_prefix.as_deref())?; + remove_new_untracked( + repo_root.as_path(), + commit.preexisting_untracked_files(), + commit.preexisting_untracked_dirs(), + current_untracked, + )?; + restore_to_commit_inner(repo_root.as_path(), repo_prefix.as_deref(), commit.id()) } /// Restore the working tree to match the given commit ID. @@ -141,7 +174,16 @@ pub fn restore_to_commit(repo_path: &Path, commit_id: &str) -> Result<(), GitToo let repo_root = resolve_repository_root(repo_path)?; let repo_prefix = repo_subdir(repo_root.as_path(), repo_path); + restore_to_commit_inner(repo_root.as_path(), repo_prefix.as_deref(), commit_id) +} +/// Restores the working tree and index to the given commit using `git restore`. +/// The repository root and optional repository-relative prefix limit the restore scope. +fn restore_to_commit_inner( + repo_root: &Path, + repo_prefix: Option<&Path>, + commit_id: &str, +) -> Result<(), GitToolingError> { let mut restore_args = vec![ OsString::from("restore"), OsString::from("--source"), @@ -150,13 +192,143 @@ pub fn restore_to_commit(repo_path: &Path, commit_id: &str) -> Result<(), GitToo OsString::from("--staged"), OsString::from("--"), ]; - if let Some(prefix) = repo_prefix.as_deref() { + if let Some(prefix) = repo_prefix { restore_args.push(prefix.as_os_str().to_os_string()); } else { restore_args.push(OsString::from(".")); } - run_git_for_status(repo_root.as_path(), restore_args, None)?; + run_git_for_status(repo_root, restore_args, None)?; + Ok(()) +} + +#[derive(Default)] +struct UntrackedSnapshot { + files: Vec, + dirs: Vec, +} + +/// Captures the untracked and ignored entries under `repo_root`, optionally limited by `repo_prefix`. +/// Returns the result as an `UntrackedSnapshot`. +fn capture_existing_untracked( + repo_root: &Path, + repo_prefix: Option<&Path>, +) -> Result { + // Ask git for the zero-delimited porcelain status so we can enumerate + // every untracked or ignored path (including ones filtered by prefix). + let mut args = vec![ + OsString::from("status"), + OsString::from("--porcelain=2"), + OsString::from("-z"), + OsString::from("--ignored=matching"), + OsString::from("--untracked-files=all"), + ]; + if let Some(prefix) = repo_prefix { + args.push(OsString::from("--")); + args.push(prefix.as_os_str().to_os_string()); + } + + let output = run_git_for_stdout_all(repo_root, args, None)?; + if output.is_empty() { + return Ok(UntrackedSnapshot::default()); + } + + let mut snapshot = UntrackedSnapshot::default(); + // Each entry is of the form " " where code is '?' (untracked) + // or '!' (ignored); everything else is irrelevant to this snapshot. + for entry in output.split('\0') { + if entry.is_empty() { + continue; + } + let mut parts = entry.splitn(2, ' '); + let code = parts.next(); + let path_part = parts.next(); + let (Some(code), Some(path_part)) = (code, path_part) else { + continue; + }; + if code != "?" && code != "!" { + continue; + } + if path_part.is_empty() { + continue; + } + + let normalized = normalize_relative_path(Path::new(path_part))?; + let absolute = repo_root.join(&normalized); + let is_dir = absolute.is_dir(); + if is_dir { + snapshot.dirs.push(normalized); + } else { + snapshot.files.push(normalized); + } + } + + Ok(snapshot) +} + +/// Removes untracked files and directories that were not present when the snapshot was captured. +fn remove_new_untracked( + repo_root: &Path, + preserved_files: &[PathBuf], + preserved_dirs: &[PathBuf], + current: UntrackedSnapshot, +) -> Result<(), GitToolingError> { + if current.files.is_empty() && current.dirs.is_empty() { + return Ok(()); + } + + let preserved_file_set: HashSet = preserved_files.iter().cloned().collect(); + let preserved_dirs_vec: Vec = preserved_dirs.to_vec(); + + for path in current.files { + if should_preserve(&path, &preserved_file_set, &preserved_dirs_vec) { + continue; + } + remove_path(&repo_root.join(&path))?; + } + + for dir in current.dirs { + if should_preserve(&dir, &preserved_file_set, &preserved_dirs_vec) { + continue; + } + remove_path(&repo_root.join(&dir))?; + } + + Ok(()) +} + +/// Determines whether an untracked path should be kept because it existed in the snapshot. +fn should_preserve( + path: &Path, + preserved_files: &HashSet, + preserved_dirs: &[PathBuf], +) -> bool { + if preserved_files.contains(path) { + return true; + } + + preserved_dirs + .iter() + .any(|dir| path.starts_with(dir.as_path())) +} + +/// Deletes the file or directory at the provided path, ignoring if it is already absent. +fn remove_path(path: &Path) -> Result<(), GitToolingError> { + match fs::symlink_metadata(path) { + Ok(metadata) => { + if metadata.is_dir() { + fs::remove_dir_all(path)?; + } else { + fs::remove_file(path)?; + } + } + Err(err) => { + if err.kind() == io::ErrorKind::NotFound { + return Ok(()); + } + return Err(err.into()); + } + } Ok(()) } @@ -239,6 +411,9 @@ mod tests { ], ); + let preexisting_untracked = repo.join("notes.txt"); + std::fs::write(&preexisting_untracked, "notes before\n")?; + let tracked_contents = "modified contents\n"; std::fs::write(repo.join("tracked.txt"), tracked_contents)?; std::fs::remove_file(repo.join("delete-me.txt"))?; @@ -267,6 +442,7 @@ mod tests { std::fs::write(repo.join("ignored.txt"), "changed\n")?; std::fs::remove_file(repo.join("new-file.txt"))?; std::fs::write(repo.join("ephemeral.txt"), "temp data\n")?; + std::fs::write(&preexisting_untracked, "notes after\n")?; restore_ghost_commit(repo, &ghost)?; @@ -277,7 +453,9 @@ mod tests { let new_file_after = std::fs::read_to_string(repo.join("new-file.txt"))?; assert_eq!(new_file_after, new_file_contents); assert_eq!(repo.join("delete-me.txt").exists(), false); - assert!(repo.join("ephemeral.txt").exists()); + assert!(!repo.join("ephemeral.txt").exists()); + let notes_after = std::fs::read_to_string(&preexisting_untracked)?; + assert_eq!(notes_after, "notes before\n"); Ok(()) } @@ -488,7 +666,43 @@ mod tests { assert!(vscode.join("settings.json").exists()); let settings_after = std::fs::read_to_string(vscode.join("settings.json"))?; assert_eq!(settings_after, "{\n \"after\": true\n}\n"); - assert!(repo.join("temp.txt").exists()); + assert!(!repo.join("temp.txt").exists()); + + Ok(()) + } + + #[test] + /// Restoring removes ignored directories created after the snapshot. + fn restore_removes_new_ignored_directory() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + + std::fs::write(repo.join(".gitignore"), ".vscode/\n")?; + std::fs::write(repo.join("tracked.txt"), "snapshot version\n")?; + run_git_in(repo, &["add", ".gitignore", "tracked.txt"]); + run_git_in( + repo, + &[ + "-c", + "user.name=Tester", + "-c", + "user.email=test@example.com", + "commit", + "-m", + "initial", + ], + ); + + let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo))?; + + let vscode = repo.join(".vscode"); + std::fs::create_dir_all(&vscode)?; + std::fs::write(vscode.join("settings.json"), "{\n \"after\": true\n}\n")?; + + restore_ghost_commit(repo, &ghost)?; + + assert!(!vscode.exists()); Ok(()) } diff --git a/codex-rs/git-tooling/src/lib.rs b/codex-rs/git-tooling/src/lib.rs index f41d104385..e6e53924b6 100644 --- a/codex-rs/git-tooling/src/lib.rs +++ b/codex-rs/git-tooling/src/lib.rs @@ -1,4 +1,5 @@ use std::fmt; +use std::path::PathBuf; mod errors; mod ghost_commits; @@ -11,18 +12,36 @@ pub use ghost_commits::create_ghost_commit; pub use ghost_commits::restore_ghost_commit; pub use ghost_commits::restore_to_commit; pub use platform::create_symlink; +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; +use ts_rs::TS; + +type CommitID = String; /// Details of a ghost commit created from a repository state. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] pub struct GhostCommit { - id: String, - parent: Option, + id: CommitID, + parent: Option, + preexisting_untracked_files: Vec, + preexisting_untracked_dirs: Vec, } impl GhostCommit { /// Create a new ghost commit wrapper from a raw commit ID and optional parent. - pub fn new(id: String, parent: Option) -> Self { - Self { id, parent } + pub fn new( + id: CommitID, + parent: Option, + preexisting_untracked_files: Vec, + preexisting_untracked_dirs: Vec, + ) -> Self { + Self { + id, + parent, + preexisting_untracked_files, + preexisting_untracked_dirs, + } } /// Commit ID for the snapshot. @@ -34,6 +53,16 @@ impl GhostCommit { pub fn parent(&self) -> Option<&str> { self.parent.as_deref() } + + /// Untracked or ignored files that already existed when the snapshot was captured. + pub fn preexisting_untracked_files(&self) -> &[PathBuf] { + &self.preexisting_untracked_files + } + + /// Untracked or ignored directories that already existed when the snapshot was captured. + pub fn preexisting_untracked_dirs(&self) -> &[PathBuf] { + &self.preexisting_untracked_dirs + } } impl fmt::Display for GhostCommit { diff --git a/codex-rs/git-tooling/src/operations.rs b/codex-rs/git-tooling/src/operations.rs index 3b387f8498..415d63d568 100644 --- a/codex-rs/git-tooling/src/operations.rs +++ b/codex-rs/git-tooling/src/operations.rs @@ -161,6 +161,27 @@ where }) } +/// Executes `git` and returns the full stdout without trimming so callers +/// can parse delimiter-sensitive output, propagating UTF-8 errors with context. +pub(crate) fn run_git_for_stdout_all( + dir: &Path, + args: I, + env: Option<&[(OsString, OsString)]>, +) -> Result +where + I: IntoIterator, + S: AsRef, +{ + // Keep the raw stdout untouched so callers can parse delimiter-sensitive + // output (e.g. NUL-separated paths) without trimming artefacts. + let run = run_git(dir, args, env)?; + // Propagate UTF-8 conversion failures with the command context for debugging. + String::from_utf8(run.output.stdout).map_err(|source| GitToolingError::GitOutputUtf8 { + command: run.command, + source, + }) +} + fn run_git( dir: &Path, args: I, diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index a59755008d..06b6b2fd59 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -286,6 +286,8 @@ async fn run_codex_tool_session_inner( | EventMsg::EnteredReviewMode(_) | EventMsg::ItemStarted(_) | EventMsg::ItemCompleted(_) + | EventMsg::UndoStarted(_) + | EventMsg::UndoCompleted(_) | EventMsg::ExitedReviewMode(_) => { // For now, we do not do anything extra for these // events. Note that diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index 0393c85427..6249d870a3 100644 --- a/codex-rs/protocol/Cargo.toml +++ b/codex-rs/protocol/Cargo.toml @@ -11,6 +11,8 @@ path = "src/lib.rs" workspace = true [dependencies] +codex-git-tooling = { workspace = true } + base64 = { workspace = true } icu_decimal = { workspace = true } icu_locale_core = { workspace = true } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 02d557535e..095d7de4de 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -9,6 +9,7 @@ use serde::ser::Serializer; use ts_rs::TS; use crate::user_input::UserInput; +use codex_git_tooling::GhostCommit; use schemars::JsonSchema; #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)] @@ -118,9 +119,7 @@ pub enum ResponseItem { }, // Generated by the harness but considered exactly as a model response. GhostSnapshot { - commit_id: String, - #[serde(default, skip_serializing_if = "Option::is_none")] - parent: Option, + ghost_commit: GhostCommit, }, #[serde(other)] Other, diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 2d293549f1..bb3be33099 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -85,14 +85,5 @@ impl SlashCommand { /// Return all built-in commands in a Vec paired with their command string. pub fn built_in_slash_commands() -> Vec<(&'static str, SlashCommand)> { - let show_beta_features = beta_features_enabled(); - - SlashCommand::iter() - .filter(|cmd| *cmd != SlashCommand::Undo || show_beta_features) - .map(|c| (c.command(), c)) - .collect() -} - -fn beta_features_enabled() -> bool { - std::env::var_os("BETA_FEATURE").is_some() + SlashCommand::iter().map(|c| (c.command(), c)).collect() } From 6b0ef9d5b2567c6c19a6b5c6f16a8fb265f50ba3 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 27 Oct 2025 10:17:46 +0000 Subject: [PATCH 11/11] Fix merge --- codex-rs/core/src/tasks/ghost_snapshot.rs | 1 - codex-rs/exec/src/event_processor_with_human_output.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/codex-rs/core/src/tasks/ghost_snapshot.rs b/codex-rs/core/src/tasks/ghost_snapshot.rs index 1f883cf454..f421a91e4e 100644 --- a/codex-rs/core/src/tasks/ghost_snapshot.rs +++ b/codex-rs/core/src/tasks/ghost_snapshot.rs @@ -10,7 +10,6 @@ use codex_protocol::models::ResponseItem; use codex_protocol::user_input::UserInput; use codex_utils_readiness::Readiness; use codex_utils_readiness::Token; -use std::borrow::ToOwned; use std::sync::Arc; use tokio_util::sync::CancellationToken; use tracing::info; 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 9ed95c5cec..2d06c881a8 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -499,7 +499,6 @@ impl EventProcessor for EventProcessorWithHumanOutput { | EventMsg::GetHistoryEntryResponse(_) | EventMsg::McpListToolsResponse(_) | EventMsg::ListCustomPromptsResponse(_) - | EventMsg::ConversationPath(_) | EventMsg::UserMessage(_) | EventMsg::EnteredReviewMode(_) | EventMsg::ExitedReviewMode(_)