diff --git a/codex-rs/app-server-protocol/src/protocol/event_mapping.rs b/codex-rs/app-server-protocol/src/protocol/event_mapping.rs index f516fc528c..809f08050f 100644 --- a/codex-rs/app-server-protocol/src/protocol/event_mapping.rs +++ b/codex-rs/app-server-protocol/src/protocol/event_mapping.rs @@ -1,7 +1,6 @@ use crate::protocol::common::ServerNotification; use crate::protocol::item_builders::build_command_execution_begin_item; use crate::protocol::item_builders::build_command_execution_end_item; -use crate::protocol::item_builders::build_file_change_begin_item; use crate::protocol::item_builders::convert_patch_changes; use crate::protocol::v2::AgentMessageDeltaNotification; use crate::protocol::v2::CollabAgentState; @@ -450,13 +449,6 @@ pub fn item_event_to_server_notification( item: item_completed_event.item.into(), }) } - EventMsg::PatchApplyBegin(patch_begin_event) => { - ServerNotification::ItemStarted(ItemStartedNotification { - thread_id, - turn_id, - item: build_file_change_begin_item(&patch_begin_event), - }) - } EventMsg::PatchApplyUpdated(event) => { ServerNotification::FileChangePatchUpdated(FileChangePatchUpdatedNotification { thread_id, diff --git a/codex-rs/app-server-protocol/src/protocol/item_builders.rs b/codex-rs/app-server-protocol/src/protocol/item_builders.rs index 546fb1b679..69ba331ce6 100644 --- a/codex-rs/app-server-protocol/src/protocol/item_builders.rs +++ b/codex-rs/app-server-protocol/src/protocol/item_builders.rs @@ -1,9 +1,8 @@ -//! Shared builders for synthetic [`ThreadItem`] values emitted by the app-server layer. +//! Shared builders for app-server [`ThreadItem`] values derived from compatibility events. //! -//! These items do not come from first-class core `ItemStarted` / `ItemCompleted` events. -//! Instead, the app-server synthesizes them so clients can render a coherent lifecycle for -//! approvals and other pre-execution flows before the underlying tool has started or when the -//! tool never starts at all. +//! Most live tool items now come from first-class core `ItemStarted` / `ItemCompleted` events. +//! These builders remain for approval flows, rebuilt legacy history, and other pre-execution +//! paths where the underlying tool has not started or never starts at all. //! //! Keeping these builders in one place is useful for two reasons: //! - Live notifications and rebuilt `thread/read` history both need to construct the same diff --git a/codex-rs/app-server-protocol/src/protocol/thread_history.rs b/codex-rs/app-server-protocol/src/protocol/thread_history.rs index c95637fe66..b1f23bb8fb 100644 --- a/codex-rs/app-server-protocol/src/protocol/thread_history.rs +++ b/codex-rs/app-server-protocol/src/protocol/thread_history.rs @@ -357,6 +357,7 @@ impl ThreadHistoryBuilder { | codex_protocol::items::TurnItem::Reasoning(_) | codex_protocol::items::TurnItem::WebSearch(_) | codex_protocol::items::TurnItem::ImageGeneration(_) + | codex_protocol::items::TurnItem::FileChange(_) | codex_protocol::items::TurnItem::ContextCompaction(_) => {} } } @@ -378,6 +379,7 @@ impl ThreadHistoryBuilder { | codex_protocol::items::TurnItem::Reasoning(_) | codex_protocol::items::TurnItem::WebSearch(_) | codex_protocol::items::TurnItem::ImageGeneration(_) + | codex_protocol::items::TurnItem::FileChange(_) | codex_protocol::items::TurnItem::ContextCompaction(_) => {} } } diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index cbcc12c3a7..fe55a8714e 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -5,6 +5,7 @@ use std::path::PathBuf; use crate::RequestId; use crate::protocol::common::AuthMode; +use crate::protocol::item_builders::convert_patch_changes; use codex_experimental_api_macros::ExperimentalApi; use codex_protocol::account::PlanType; use codex_protocol::account::ProviderAccount; @@ -6469,6 +6470,15 @@ impl From for ThreadItem { result: image.result, saved_path: image.saved_path, }, + CoreTurnItem::FileChange(file_change) => ThreadItem::FileChange { + id: file_change.id, + changes: convert_patch_changes(&file_change.changes), + status: file_change + .status + .as_ref() + .map(PatchApplyStatus::from) + .unwrap_or(PatchApplyStatus::InProgress), + }, CoreTurnItem::ContextCompaction(compaction) => { ThreadItem::ContextCompaction { id: compaction.id } } @@ -8078,6 +8088,7 @@ mod tests { use super::*; use codex_protocol::items::AgentMessageContent; use codex_protocol::items::AgentMessageItem; + use codex_protocol::items::FileChangeItem; use codex_protocol::items::ReasoningItem; use codex_protocol::items::TurnItem; use codex_protocol::items::UserMessageItem; @@ -10358,6 +10369,35 @@ mod tests { }), } ); + + let file_change_item = TurnItem::FileChange(FileChangeItem { + id: "patch-1".to_string(), + changes: [( + PathBuf::from("README.md"), + codex_protocol::protocol::FileChange::Add { + content: "hello\n".to_string(), + }, + )] + .into_iter() + .collect(), + status: Some(codex_protocol::protocol::PatchApplyStatus::Completed), + auto_approved: None, + stdout: Some("Done!".to_string()), + stderr: Some(String::new()), + }); + + assert_eq!( + ThreadItem::from(file_change_item), + ThreadItem::FileChange { + id: "patch-1".to_string(), + changes: vec![FileUpdateChange { + path: "README.md".to_string(), + kind: PatchChangeKind::Add, + diff: "hello\n".to_string(), + }], + status: PatchApplyStatus::Completed, + } + ); } #[test] diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 628034da72..bb77a71705 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -29,7 +29,6 @@ use codex_app_server_protocol::ExecPolicyAmendment as V2ExecPolicyAmendment; use codex_app_server_protocol::FileChangeApprovalDecision; use codex_app_server_protocol::FileChangeRequestApprovalParams; use codex_app_server_protocol::FileChangeRequestApprovalResponse; -use codex_app_server_protocol::FileUpdateChange; use codex_app_server_protocol::GrantedPermissionProfile as V2GrantedPermissionProfile; use codex_app_server_protocol::GuardianWarningNotification; use codex_app_server_protocol::HookCompletedNotification; @@ -46,7 +45,6 @@ use codex_app_server_protocol::ModelVerificationNotification; use codex_app_server_protocol::NetworkApprovalContext as V2NetworkApprovalContext; use codex_app_server_protocol::NetworkPolicyAmendment as V2NetworkPolicyAmendment; use codex_app_server_protocol::NetworkPolicyRuleAction as V2NetworkPolicyRuleAction; -use codex_app_server_protocol::PatchApplyStatus; use codex_app_server_protocol::PermissionsRequestApprovalParams; use codex_app_server_protocol::PermissionsRequestApprovalResponse; use codex_app_server_protocol::RawResponseItemCompletedNotification; @@ -82,11 +80,8 @@ use codex_app_server_protocol::TurnPlanUpdatedNotification; use codex_app_server_protocol::TurnStartedNotification; use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::WarningNotification; -use codex_app_server_protocol::build_file_change_approval_request_item; -use codex_app_server_protocol::build_file_change_end_item; use codex_app_server_protocol::build_item_from_guardian_event; use codex_app_server_protocol::build_turns_from_rollout_items; -use codex_app_server_protocol::convert_patch_changes; use codex_app_server_protocol::guardian_auto_approval_review_notification; use codex_app_server_protocol::item_event_to_server_notification; use codex_core::CodexThread; @@ -524,28 +519,7 @@ pub(crate) async fn apply_bespoke_event_handling( let permission_guard = thread_watch_manager .note_permission_requested(&conversation_id.to_string()) .await; - // Until we migrate the core to be aware of a first class FileChangeItem - // and emit the corresponding EventMsg, we repurpose the call_id as the item_id. let item_id = event.call_id.clone(); - let patch_changes = convert_patch_changes(&event.changes); - let first_start = { - let mut state = thread_state.lock().await; - state - .turn_summary - .file_change_started - .insert(item_id.clone()) - }; - if first_start { - let item = build_file_change_approval_request_item(&event); - let notification = ItemStartedNotification { - thread_id: conversation_id.to_string(), - turn_id: event_turn_id.clone(), - item, - }; - outgoing - .send_server_notification(ServerNotification::ItemStarted(notification)) - .await; - } let params = FileChangeRequestApprovalParams { thread_id: conversation_id.to_string(), @@ -559,14 +533,10 @@ pub(crate) async fn apply_bespoke_event_handling( .await; tokio::spawn(async move { on_file_change_request_approval_response( - event_turn_id, - conversation_id, item_id, - patch_changes, pending_request_id, rx, conversation, - outgoing, thread_state.clone(), permission_guard, ) @@ -1104,40 +1074,9 @@ pub(crate) async fn apply_bespoke_event_handling( ) .await; } - EventMsg::PatchApplyBegin(patch_begin_event) => { - // Until we migrate the core to be aware of a first class FileChangeItem - // and emit the corresponding EventMsg, we repurpose the call_id as the item_id. - let item_id = patch_begin_event.call_id.clone(); - - let first_start = { - let mut state = thread_state.lock().await; - state - .turn_summary - .file_change_started - .insert(item_id.clone()) - }; - if first_start { - let notification = item_event_to_server_notification( - EventMsg::PatchApplyBegin(patch_begin_event), - &conversation_id.to_string(), - &event_turn_id, - ); - outgoing.send_server_notification(notification).await; - } - } - EventMsg::PatchApplyEnd(patch_end_event) => { - // Until we migrate the core to be aware of a first class FileChangeItem - // and emit the corresponding EventMsg, we repurpose the call_id as the item_id. - let item_id = patch_end_event.call_id.clone(); - complete_file_change_item( - conversation_id, - item_id, - build_file_change_end_item(&patch_end_event), - event_turn_id.clone(), - &outgoing, - &thread_state, - ) - .await; + EventMsg::PatchApplyBegin(_) | EventMsg::PatchApplyEnd(_) => { + // Core still fans out these deprecated events for legacy clients; + // v2 clients receive the canonical FileChange item instead. } EventMsg::ExecCommandBegin(exec_command_begin_event) => { if matches!( @@ -1425,31 +1364,6 @@ async fn emit_turn_completed_with_status( .await; } -async fn complete_file_change_item( - conversation_id: ThreadId, - item_id: String, - item: ThreadItem, - turn_id: String, - outgoing: &ThreadScopedOutgoingMessageSender, - thread_state: &Arc>, -) { - thread_state - .lock() - .await - .turn_summary - .file_change_started - .remove(&item_id); - - let notification = ItemCompletedNotification { - thread_id: conversation_id.to_string(), - turn_id, - item, - }; - outgoing - .send_server_notification(ServerNotification::ItemCompleted(notification)) - .await; -} - #[allow(clippy::too_many_arguments)] async fn start_command_execution_item( conversation_id: &ThreadId, @@ -2002,38 +1916,28 @@ fn render_review_output_text(output: &ReviewOutputEvent) -> String { } } -fn map_file_change_approval_decision( - decision: FileChangeApprovalDecision, -) -> (ReviewDecision, Option) { +fn map_file_change_approval_decision(decision: FileChangeApprovalDecision) -> ReviewDecision { match decision { - FileChangeApprovalDecision::Accept => (ReviewDecision::Approved, None), - FileChangeApprovalDecision::AcceptForSession => (ReviewDecision::ApprovedForSession, None), - FileChangeApprovalDecision::Decline => { - (ReviewDecision::Denied, Some(PatchApplyStatus::Declined)) - } - FileChangeApprovalDecision::Cancel => { - (ReviewDecision::Abort, Some(PatchApplyStatus::Declined)) - } + FileChangeApprovalDecision::Accept => ReviewDecision::Approved, + FileChangeApprovalDecision::AcceptForSession => ReviewDecision::ApprovedForSession, + FileChangeApprovalDecision::Decline => ReviewDecision::Denied, + FileChangeApprovalDecision::Cancel => ReviewDecision::Abort, } } #[allow(clippy::too_many_arguments)] async fn on_file_change_request_approval_response( - event_turn_id: String, - conversation_id: ThreadId, item_id: String, - changes: Vec, pending_request_id: RequestId, receiver: oneshot::Receiver, codex: Arc, - outgoing: ThreadScopedOutgoingMessageSender, thread_state: Arc>, permission_guard: ThreadWatchActiveGuard, ) { let response = receiver.await; resolve_server_request_on_thread_listener(&thread_state, pending_request_id).await; drop(permission_guard); - let (decision, completion_status) = match response { + let decision = match response { Ok(Ok(value)) => { let response = serde_json::from_value::(value) .unwrap_or_else(|err| { @@ -2043,39 +1947,19 @@ async fn on_file_change_request_approval_response( } }); - let (decision, completion_status) = - map_file_change_approval_decision(response.decision); - // Allow EventMsg::PatchApplyEnd to emit ItemCompleted for accepted patches. - // Only short-circuit on declines/cancels/failures. - (decision, completion_status) + map_file_change_approval_decision(response.decision) } Ok(Err(err)) if is_turn_transition_server_request_error(&err) => return, Ok(Err(err)) => { error!("request failed with client error: {err:?}"); - (ReviewDecision::Denied, Some(PatchApplyStatus::Failed)) + ReviewDecision::Denied } Err(err) => { error!("request failed: {err:?}"); - (ReviewDecision::Denied, Some(PatchApplyStatus::Failed)) + ReviewDecision::Denied } }; - if let Some(status) = completion_status { - complete_file_change_item( - conversation_id, - item_id.clone(), - ThreadItem::FileChange { - id: item_id.clone(), - changes, - status, - }, - event_turn_id.clone(), - &outgoing, - &thread_state, - ) - .await; - } - if let Err(err) = codex .submit(Op::PatchApproval { id: item_id, @@ -2886,10 +2770,9 @@ mod tests { #[test] fn file_change_accept_for_session_maps_to_approved_for_session() { - let (decision, completion_status) = + let decision = map_file_change_approval_decision(FileChangeApprovalDecision::AcceptForSession); assert_eq!(decision, ReviewDecision::ApprovedForSession); - assert_eq!(completion_status, None); } #[test] diff --git a/codex-rs/app-server/src/thread_state.rs b/codex-rs/app-server/src/thread_state.rs index 5122334843..dddbcf483b 100644 --- a/codex-rs/app-server/src/thread_state.rs +++ b/codex-rs/app-server/src/thread_state.rs @@ -61,7 +61,6 @@ pub(crate) enum ThreadListenerCommand { #[derive(Default, Clone)] pub(crate) struct TurnSummary { pub(crate) started_at: Option, - pub(crate) file_change_started: HashSet, pub(crate) command_execution_started: HashSet, pub(crate) last_error: Option, } diff --git a/codex-rs/core/src/tools/events.rs b/codex-rs/core/src/tools/events.rs index 2b215a043d..6469a4984e 100644 --- a/codex-rs/core/src/tools/events.rs +++ b/codex-rs/core/src/tools/events.rs @@ -6,6 +6,8 @@ use crate::tools::sandboxing::ToolError; use codex_protocol::error::CodexErr; use codex_protocol::error::SandboxErr; use codex_protocol::exec_output::ExecToolCallOutput; +use codex_protocol::items::FileChangeItem; +use codex_protocol::items::TurnItem; use codex_protocol::parse_command::ParsedCommand; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecCommandBeginEvent; @@ -13,8 +15,6 @@ use codex_protocol::protocol::ExecCommandEndEvent; use codex_protocol::protocol::ExecCommandSource; use codex_protocol::protocol::ExecCommandStatus; use codex_protocol::protocol::FileChange; -use codex_protocol::protocol::PatchApplyBeginEvent; -use codex_protocol::protocol::PatchApplyEndEvent; use codex_protocol::protocol::PatchApplyStatus; use codex_protocol::protocol::TurnDiffEvent; use codex_shell_command::parse_command::parse_command; @@ -183,13 +183,15 @@ impl ToolEmitter { guard.on_patch_begin(changes); } ctx.session - .send_event( + .emit_turn_item_started( ctx.turn, - EventMsg::PatchApplyBegin(PatchApplyBeginEvent { - call_id: ctx.call_id.to_string(), - turn_id: ctx.turn.sub_id.clone(), - auto_approved: *auto_approved, + &TurnItem::FileChange(FileChangeItem { + id: ctx.call_id.to_string(), changes: changes.clone(), + status: None, + auto_approved: Some(*auto_approved), + stdout: None, + stderr: None, }), ) .await; @@ -200,7 +202,6 @@ impl ToolEmitter { changes.clone(), output.stdout.text.clone(), output.stderr.text.clone(), - output.exit_code == 0, if output.exit_code == 0 { PatchApplyStatus::Completed } else { @@ -218,7 +219,6 @@ impl ToolEmitter { changes.clone(), output.stdout.text.clone(), output.stderr.text.clone(), - output.exit_code == 0, if output.exit_code == 0 { PatchApplyStatus::Completed } else { @@ -236,7 +236,6 @@ impl ToolEmitter { changes.clone(), String::new(), (*message).to_string(), - /*success*/ false, PatchApplyStatus::Failed, ) .await; @@ -250,7 +249,6 @@ impl ToolEmitter { changes.clone(), String::new(), (*message).to_string(), - /*success*/ false, PatchApplyStatus::Declined, ) .await; @@ -496,20 +494,18 @@ async fn emit_patch_end( changes: HashMap, stdout: String, stderr: String, - success: bool, status: PatchApplyStatus, ) { ctx.session - .send_event( + .emit_turn_item_completed( ctx.turn, - EventMsg::PatchApplyEnd(PatchApplyEndEvent { - call_id: ctx.call_id.to_string(), - turn_id: ctx.turn.sub_id.clone(), - stdout, - stderr, - success, + TurnItem::FileChange(FileChangeItem { + id: ctx.call_id.to_string(), changes, - status, + status: Some(status), + auto_approved: None, + stdout: Some(stdout), + stderr: Some(stderr), }), ) .await; diff --git a/codex-rs/core/tests/suite/tool_harness.rs b/codex-rs/core/tests/suite/tool_harness.rs index 62d6dcef90..a69ec3f7f6 100644 --- a/codex-rs/core/tests/suite/tool_harness.rs +++ b/codex-rs/core/tests/suite/tool_harness.rs @@ -4,6 +4,7 @@ use std::fs; use assert_matches::assert_matches; use codex_features::Feature; +use codex_protocol::items::TurnItem; use codex_protocol::models::PermissionProfile; use codex_protocol::plan_tool::StepStatus; use codex_protocol::protocol::AskForApproval; @@ -365,9 +366,30 @@ async fn apply_patch_tool_executes_and_emits_patch_events() -> anyhow::Result<() }) .await?; + let mut saw_file_change_started = false; + let mut saw_file_change_completed = false; let mut saw_patch_begin = false; let mut patch_end_success = None; wait_for_event(&codex, |event| match event { + EventMsg::ItemStarted(started) => { + if let TurnItem::FileChange(item) = &started.item { + saw_file_change_started = true; + assert_eq!(item.id, call_id); + assert_eq!(item.status, None); + } + false + } + EventMsg::ItemCompleted(completed) => { + if let TurnItem::FileChange(item) = &completed.item { + saw_file_change_completed = true; + assert_eq!(item.id, call_id); + assert_eq!( + item.status, + Some(codex_protocol::protocol::PatchApplyStatus::Completed) + ); + } + false + } EventMsg::PatchApplyBegin(begin) => { saw_patch_begin = true; assert_eq!(begin.call_id, call_id); @@ -383,6 +405,14 @@ async fn apply_patch_tool_executes_and_emits_patch_events() -> anyhow::Result<() }) .await; + assert!( + saw_file_change_started, + "expected ItemStarted for TurnItem::FileChange" + ); + assert!( + saw_file_change_completed, + "expected ItemCompleted for TurnItem::FileChange" + ); assert!(saw_patch_begin, "expected PatchApplyBegin event"); let patch_end_success = patch_end_success.expect("expected PatchApplyEnd event to capture success flag"); diff --git a/codex-rs/protocol/src/items.rs b/codex-rs/protocol/src/items.rs index 6879588579..f9c0bd5882 100644 --- a/codex-rs/protocol/src/items.rs +++ b/codex-rs/protocol/src/items.rs @@ -8,7 +8,11 @@ use crate::protocol::AgentReasoningEvent; use crate::protocol::AgentReasoningRawContentEvent; use crate::protocol::ContextCompactedEvent; use crate::protocol::EventMsg; +use crate::protocol::FileChange; use crate::protocol::ImageGenerationEndEvent; +use crate::protocol::PatchApplyBeginEvent; +use crate::protocol::PatchApplyEndEvent; +use crate::protocol::PatchApplyStatus; use crate::protocol::UserMessageEvent; use crate::protocol::WebSearchEndEvent; use crate::user_input::ByteRange; @@ -20,6 +24,8 @@ use quick_xml::se::to_string as to_xml_string; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; +use std::collections::HashMap; +use std::path::PathBuf; use ts_rs::TS; #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)] @@ -33,6 +39,7 @@ pub enum TurnItem { Reasoning(ReasoningItem), WebSearch(WebSearchItem), ImageGeneration(ImageGenerationItem), + FileChange(FileChangeItem), ContextCompaction(ContextCompactionItem), } @@ -127,6 +134,24 @@ pub struct ImageGenerationItem { pub saved_path: Option, } +#[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema, PartialEq)] +pub struct FileChangeItem { + pub id: String, + pub changes: HashMap, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub status: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub auto_approved: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub stdout: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub stderr: Option, +} + #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)] pub struct ContextCompactionItem { pub id: String, @@ -381,6 +406,30 @@ impl ImageGenerationItem { } } +impl FileChangeItem { + pub fn as_legacy_begin_event(&self, turn_id: String) -> EventMsg { + EventMsg::PatchApplyBegin(PatchApplyBeginEvent { + call_id: self.id.clone(), + turn_id, + auto_approved: self.auto_approved.unwrap_or(false), + changes: self.changes.clone(), + }) + } + + pub fn as_legacy_end_event(&self, turn_id: String) -> Option { + let status = self.status.clone()?; + Some(EventMsg::PatchApplyEnd(PatchApplyEndEvent { + call_id: self.id.clone(), + turn_id, + stdout: self.stdout.clone().unwrap_or_default(), + stderr: self.stderr.clone().unwrap_or_default(), + success: status == PatchApplyStatus::Completed, + changes: self.changes.clone(), + status, + })) + } +} + impl TurnItem { pub fn id(&self) -> String { match self { @@ -391,6 +440,7 @@ impl TurnItem { TurnItem::Reasoning(item) => item.id.clone(), TurnItem::WebSearch(item) => item.id.clone(), TurnItem::ImageGeneration(item) => item.id.clone(), + TurnItem::FileChange(item) => item.id.clone(), TurnItem::ContextCompaction(item) => item.id.clone(), } } @@ -403,6 +453,10 @@ impl TurnItem { TurnItem::Plan(_) => Vec::new(), TurnItem::WebSearch(item) => vec![item.as_legacy_event()], TurnItem::ImageGeneration(item) => vec![item.as_legacy_event()], + TurnItem::FileChange(item) => item + .as_legacy_end_event(String::new()) + .into_iter() + .collect(), TurnItem::Reasoning(item) => item.as_legacy_events(show_raw_agent_reasoning), TurnItem::ContextCompaction(item) => vec![item.as_legacy_event()], } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index c3e4f5abaa..f4b3a52d97 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1841,6 +1841,7 @@ impl HasLegacyEvent for ItemStartedEvent { call_id: item.id.clone(), })] } + TurnItem::FileChange(item) => vec![item.as_legacy_begin_event(self.turn_id.clone())], _ => Vec::new(), } } @@ -1859,7 +1860,13 @@ pub trait HasLegacyEvent { impl HasLegacyEvent for ItemCompletedEvent { fn as_legacy_events(&self, show_raw_agent_reasoning: bool) -> Vec { - self.item.as_legacy_events(show_raw_agent_reasoning) + match &self.item { + TurnItem::FileChange(item) => item + .as_legacy_end_event(self.turn_id.clone()) + .into_iter() + .collect(), + _ => self.item.as_legacy_events(show_raw_agent_reasoning), + } } } @@ -3928,6 +3935,7 @@ pub struct CollabResumeEndEvent { #[cfg(test)] mod tests { use super::*; + use crate::items::FileChangeItem; use crate::items::ImageGenerationItem; use crate::items::UserMessageItem; use crate::items::WebSearchItem; @@ -4630,6 +4638,41 @@ mod tests { } } + #[test] + fn item_started_event_from_file_change_emits_patch_begin_event() { + let event = ItemStartedEvent { + thread_id: ThreadId::new(), + turn_id: "turn-1".into(), + item: TurnItem::FileChange(FileChangeItem { + id: "patch-1".into(), + changes: [( + PathBuf::from("new.txt"), + FileChange::Add { + content: "hello".into(), + }, + )] + .into_iter() + .collect(), + status: None, + auto_approved: Some(true), + stdout: None, + stderr: None, + }), + }; + + let legacy_events = event.as_legacy_events(/*show_raw_agent_reasoning*/ false); + assert_eq!(legacy_events.len(), 1); + match &legacy_events[0] { + EventMsg::PatchApplyBegin(event) => { + assert_eq!(event.call_id, "patch-1"); + assert_eq!(event.turn_id, "turn-1"); + assert!(event.auto_approved); + assert!(event.changes.contains_key(&PathBuf::from("new.txt"))); + } + _ => panic!("expected PatchApplyBegin event"), + } + } + #[test] fn item_completed_event_from_image_generation_emits_end_event() { let event = ItemCompletedEvent { @@ -4661,6 +4704,43 @@ mod tests { } } + #[test] + fn item_completed_event_from_file_change_emits_patch_end_event() { + let event = ItemCompletedEvent { + thread_id: ThreadId::new(), + turn_id: "turn-1".into(), + item: TurnItem::FileChange(FileChangeItem { + id: "patch-1".into(), + changes: [( + PathBuf::from("new.txt"), + FileChange::Add { + content: "hello".into(), + }, + )] + .into_iter() + .collect(), + status: Some(PatchApplyStatus::Completed), + auto_approved: None, + stdout: Some("Done!".into()), + stderr: Some(String::new()), + }), + }; + + let legacy_events = event.as_legacy_events(/*show_raw_agent_reasoning*/ false); + assert_eq!(legacy_events.len(), 1); + match &legacy_events[0] { + EventMsg::PatchApplyEnd(event) => { + assert_eq!(event.call_id, "patch-1"); + assert_eq!(event.turn_id, "turn-1"); + assert_eq!(event.stdout, "Done!"); + assert!(event.success); + assert_eq!(event.status, PatchApplyStatus::Completed); + assert!(event.changes.contains_key(&PathBuf::from("new.txt"))); + } + _ => panic!("expected PatchApplyEnd event"), + } + } + #[test] fn rollback_failed_error_does_not_affect_turn_status() { let event = ErrorEvent {