diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 9b5fad7618..b44cd27b9f 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1114,6 +1114,7 @@ dependencies = [ "similar", "strum_macros 0.27.2", "tempfile", + "test-case", "test-log", "thiserror 2.0.17", "time", @@ -6160,6 +6161,39 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f50febec83f5ee1df3015341d8bd429f2d1cc62bcba7ea2076759d315084683" +[[package]] +name = "test-case" +version = "3.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eb2550dd13afcd286853192af8601920d959b14c401fcece38071d53bf0768a8" +dependencies = [ + "test-case-macros", +] + +[[package]] +name = "test-case-core" +version = "3.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "adcb7fd841cd518e279be3d5a3eb0636409487998a4aff22f3de87b81e88384f" +dependencies = [ + "cfg-if", + "proc-macro2", + "quote", + "syn 2.0.104", +] + +[[package]] +name = "test-case-macros" +version = "3.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c89e72a01ed4c579669add59014b9a524d609c0c88c6a585ce37485879f6ffb" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.104", + "test-case-core", +] + [[package]] name = "test-log" version = "0.2.18" diff --git a/codex-rs/app-server-protocol/src/export.rs b/codex-rs/app-server-protocol/src/export.rs index a1408d61d5..11296e8e54 100644 --- a/codex-rs/app-server-protocol/src/export.rs +++ b/codex-rs/app-server-protocol/src/export.rs @@ -13,10 +13,7 @@ use crate::export_server_responses; use anyhow::Context; use anyhow::Result; use anyhow::anyhow; -use codex_protocol::parse_command::ParsedCommand; use codex_protocol::protocol::EventMsg; -use codex_protocol::protocol::FileChange; -use codex_protocol::protocol::SandboxPolicy; use schemars::JsonSchema; use schemars::schema_for; use serde::Serialize; @@ -120,10 +117,6 @@ pub fn generate_json(out_dir: &Path) -> Result<()> { |d| write_json_schema_with_return::(d, "ClientNotification"), |d| write_json_schema_with_return::(d, "ServerNotification"), |d| write_json_schema_with_return::(d, "EventMsg"), - |d| write_json_schema_with_return::(d, "FileChange"), - |d| write_json_schema_with_return::(d, "InputItem"), - |d| write_json_schema_with_return::(d, "ParsedCommand"), - |d| write_json_schema_with_return::(d, "SandboxPolicy"), ]; let mut schemas: Vec = Vec::new(); @@ -152,13 +145,10 @@ fn build_schema_bundle(schemas: Vec) -> Result { "ClientNotification", "ClientRequest", "EventMsg", - "FileChange", - "InputItem", - "ParsedCommand", - "SandboxPolicy", "ServerNotification", "ServerRequest", ]; + const IGNORED_DEFINITIONS: &[&str] = &["Option<()>"]; let namespaced_types = collect_namespaced_types(&schemas); let mut definitions = Map::new(); @@ -171,6 +161,10 @@ fn build_schema_bundle(schemas: Vec) -> Result { in_v1_dir, } = schema; + if IGNORED_DEFINITIONS.contains(&logical_name.as_str()) { + continue; + } + if let Some(ref ns) = namespace { rewrite_refs_to_namespace(&mut value, ns); } @@ -181,6 +175,9 @@ fn build_schema_bundle(schemas: Vec) -> Result { && let Value::Object(defs_obj) = defs { for (def_name, mut def_schema) in defs_obj { + if IGNORED_DEFINITIONS.contains(&def_name.as_str()) { + continue; + } if SPECIAL_DEFINITIONS.contains(&def_name.as_str()) { continue; } @@ -386,14 +383,6 @@ fn variant_definition_name(base: &str, variant: &Value) -> Option { }); } - if let Some(mode_literal) = literal_from_property(props, "mode") { - let pascal = to_pascal_case(mode_literal); - return Some(match base { - "SandboxPolicy" => format!("{pascal}SandboxPolicy"), - _ => format!("{pascal}{base}"), - }); - } - if props.len() == 1 && let Some(key) = props.keys().next() { diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index a782762e50..1aac6d462b 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -493,6 +493,9 @@ server_notification_definitions! { McpToolCallProgress => "item/mcpToolCall/progress" (v2::McpToolCallProgressNotification), AccountUpdated => "account/updated" (v2::AccountUpdatedNotification), AccountRateLimitsUpdated => "account/rateLimits/updated" (v2::AccountRateLimitsUpdatedNotification), + ReasoningSummaryTextDelta => "item/reasoning/summaryTextDelta" (v2::ReasoningSummaryTextDeltaNotification), + ReasoningSummaryPartAdded => "item/reasoning/summaryPartAdded" (v2::ReasoningSummaryPartAddedNotification), + ReasoningTextDelta => "item/reasoning/textDelta" (v2::ReasoningTextDeltaNotification), #[serde(rename = "account/login/completed")] #[ts(rename = "account/login/completed")] diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index d2e5a7f43f..029ff37543 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -77,8 +77,8 @@ v2_enum_from_core!( ); #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] -#[serde(tag = "mode", rename_all = "camelCase")] -#[ts(tag = "mode")] +#[serde(tag = "type", rename_all = "camelCase")] +#[ts(tag = "type")] #[ts(export_to = "v2/")] pub enum SandboxPolicy { DangerFullAccess, @@ -661,7 +661,10 @@ pub enum ThreadItem { }, Reasoning { id: String, - text: String, + #[serde(default)] + summary: Vec, + #[serde(default)] + content: Vec, }, CommandExecution { id: String, @@ -730,17 +733,11 @@ impl From for ThreadItem { .collect::(); ThreadItem::AgentMessage { id: agent.id, text } } - CoreTurnItem::Reasoning(reasoning) => { - let text = if !reasoning.summary_text.is_empty() { - reasoning.summary_text.join("\n") - } else { - reasoning.raw_content.join("\n") - }; - ThreadItem::Reasoning { - id: reasoning.id, - text, - } - } + CoreTurnItem::Reasoning(reasoning) => ThreadItem::Reasoning { + id: reasoning.id, + summary: reasoning.summary_text, + content: reasoning.raw_content, + }, CoreTurnItem::WebSearch(search) => ThreadItem::WebSearch { id: search.id, query: search.query, @@ -874,6 +871,32 @@ pub struct AgentMessageDeltaNotification { pub delta: String, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ReasoningSummaryTextDeltaNotification { + pub item_id: String, + pub delta: String, + pub summary_index: i64, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ReasoningSummaryPartAddedNotification { + pub item_id: String, + pub summary_index: i64, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ReasoningTextDeltaNotification { + pub item_id: String, + pub delta: String, + pub content_index: i64, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -1076,7 +1099,8 @@ mod tests { ThreadItem::from(reasoning_item), ThreadItem::Reasoning { id: "reasoning-1".to_string(), - text: "line one\nline two".to_string(), + summary: vec!["line one".to_string(), "line two".to_string()], + content: vec![], } ); diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index ecbcb4b0a8..f40a860129 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -12,6 +12,7 @@ use codex_app_server_protocol::AccountRateLimitsUpdatedNotification; use codex_app_server_protocol::AccountUpdatedNotification; use codex_app_server_protocol::AddConversationListenerParams; use codex_app_server_protocol::AddConversationSubscriptionResponse; +use codex_app_server_protocol::AgentMessageDeltaNotification; use codex_app_server_protocol::ApplyPatchApprovalParams; use codex_app_server_protocol::ApplyPatchApprovalResponse; use codex_app_server_protocol::ArchiveConversationParams; @@ -70,6 +71,9 @@ use codex_app_server_protocol::ModelListResponse; use codex_app_server_protocol::NewConversationParams; use codex_app_server_protocol::NewConversationResponse; use codex_app_server_protocol::ParsedCommand as V2ParsedCommand; +use codex_app_server_protocol::ReasoningSummaryPartAddedNotification; +use codex_app_server_protocol::ReasoningSummaryTextDeltaNotification; +use codex_app_server_protocol::ReasoningTextDeltaNotification; use codex_app_server_protocol::RemoveConversationListenerParams; use codex_app_server_protocol::RemoveConversationSubscriptionResponse; use codex_app_server_protocol::RequestId; @@ -2722,6 +2726,48 @@ async fn apply_bespoke_event_handling( }); } }, + EventMsg::AgentMessageContentDelta(event) => { + let notification = AgentMessageDeltaNotification { + item_id: event.item_id, + delta: event.delta, + }; + outgoing + .send_server_notification(ServerNotification::AgentMessageDelta(notification)) + .await; + } + EventMsg::ReasoningContentDelta(event) => { + let notification = ReasoningSummaryTextDeltaNotification { + item_id: event.item_id, + delta: event.delta, + summary_index: event.summary_index, + }; + outgoing + .send_server_notification(ServerNotification::ReasoningSummaryTextDelta( + notification, + )) + .await; + } + EventMsg::ReasoningRawContentDelta(event) => { + let notification = ReasoningTextDeltaNotification { + item_id: event.item_id, + delta: event.delta, + content_index: event.content_index, + }; + outgoing + .send_server_notification(ServerNotification::ReasoningTextDelta(notification)) + .await; + } + EventMsg::AgentReasoningSectionBreak(event) => { + let notification = ReasoningSummaryPartAddedNotification { + item_id: event.item_id, + summary_index: event.summary_index, + }; + outgoing + .send_server_notification(ServerNotification::ReasoningSummaryPartAdded( + notification, + )) + .await; + } EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { call_id, turn_id, @@ -2798,6 +2844,37 @@ async fn apply_bespoke_event_handling( .send_server_notification(ServerNotification::ItemCompleted(notification)) .await; } + EventMsg::ExecCommandBegin(exec_command_begin_event) => { + let item: ThreadItem = ThreadItem::CommandExecution { + id: exec_command_begin_event.call_id.clone(), + command: exec_command_begin_event.command, + cwd: exec_command_begin_event.cwd, + status: CommandExecutionStatus::InProgress, + parsed_cmd: exec_command_begin_event.parsed_cmd, + is_user_shell_command: exec_command_begin_event.is_user_shell_command, + aggregated_output: None, + exit_code: None, + duration_ms: None, + } + .into(); + let notification = ItemStartedNotification { item }; + outgoing + .send_server_notification(ServerNotification::ItemStarted(notification)) + .await; + } + EventMsg::ExecCommandOutputDelta(exec_command_output_delta_event) => { + let notification = ExecCommandOutputDeltaNotification { + item_id: exec_command_output_delta_event.item_id, + delta: exec_command_output_delta_event.delta, + }; + outgoing + .send_server_notification(ServerNotification::ExecCommandOutputDelta(notification)) + .await; + } + EventMsg::ExecCommandEnd(exec_command_end_event) => { + // TODO: update the item to include the exit code and duration + // outgoing.send_server_notification(ServerNotification::ItemCompleted(notification)).await; + } // If this is a TurnAborted, reply to any pending interrupt requests. EventMsg::TurnAborted(turn_aborted_event) => { let pending = { diff --git a/codex-rs/app-server/tests/suite/v2/thread_list.rs b/codex-rs/app-server/tests/suite/v2/thread_list.rs index 09ef0ebfd2..464fb4eee8 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_list.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_list.rs @@ -6,10 +6,8 @@ use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ThreadListParams; use codex_app_server_protocol::ThreadListResponse; -use serde_json::json; use tempfile::TempDir; use tokio::time::timeout; -use uuid::Uuid; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); @@ -150,46 +148,13 @@ async fn thread_list_respects_provider_filter() -> Result<()> { "X", Some("mock_provider"), )?; // mock_provider - // one with a different provider - let uuid = Uuid::new_v4(); - let dir = codex_home - .path() - .join("sessions") - .join("2025") - .join("01") - .join("02"); - std::fs::create_dir_all(&dir)?; - let file_path = dir.join(format!("rollout-2025-01-02T11-00-00-{uuid}.jsonl")); - let lines = [ - json!({ - "timestamp": "2025-01-02T11:00:00Z", - "type": "session_meta", - "payload": { - "id": uuid, - "timestamp": "2025-01-02T11:00:00Z", - "cwd": "/", - "originator": "codex", - "cli_version": "0.0.0", - "instructions": null, - "source": "vscode", - "model_provider": "other_provider" - } - }) - .to_string(), - json!({ - "timestamp": "2025-01-02T11:00:00Z", - "type":"response_item", - "payload": {"type":"message","role":"user","content":[{"type":"input_text","text":"X"}]} - }) - .to_string(), - json!({ - "timestamp": "2025-01-02T11:00:00Z", - "type":"event_msg", - "payload": {"type":"user_message","message":"X","kind":"plain"} - }) - .to_string(), - ]; - std::fs::write(file_path, lines.join("\n") + "\n")?; + let _b = create_fake_rollout( + codex_home.path(), + "2025-01-02T11-00-00", + "2025-01-02T11:00:00Z", + "X", + Some("other_provider"), + )?; let mut mcp = McpProcess::new(codex_home.path()).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 6839504be5..ab732c910c 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -60,6 +60,7 @@ shlex = { workspace = true } similar = { workspace = true } strum_macros = { workspace = true } tempfile = { workspace = true } +test-case = "3.3.1" test-log = { workspace = true } thiserror = { workspace = true } time = { workspace = true, features = [ diff --git a/codex-rs/core/src/chat_completions.rs b/codex-rs/core/src/chat_completions.rs index d8d3a63d13..5b9578e759 100644 --- a/codex-rs/core/src/chat_completions.rs +++ b/codex-rs/core/src/chat_completions.rs @@ -477,10 +477,14 @@ async fn append_reasoning_text( .. }) = reasoning_item { + let content_index = content.len() as i64; content.push(ReasoningItemContent::ReasoningText { text: text.clone() }); let _ = tx_event - .send(Ok(ResponseEvent::ReasoningContentDelta(text.clone()))) + .send(Ok(ResponseEvent::ReasoningContentDelta { + delta: text.clone(), + content_index, + })) .await; } } @@ -898,20 +902,26 @@ where continue; } } - Poll::Ready(Some(Ok(ResponseEvent::ReasoningContentDelta(delta)))) => { + Poll::Ready(Some(Ok(ResponseEvent::ReasoningContentDelta { + delta, + content_index, + }))) => { // Always accumulate reasoning deltas so we can emit a final Reasoning item at Completed. this.cumulative_reasoning.push_str(&delta); if matches!(this.mode, AggregateMode::Streaming) { // In streaming mode, also forward the delta immediately. - return Poll::Ready(Some(Ok(ResponseEvent::ReasoningContentDelta(delta)))); + return Poll::Ready(Some(Ok(ResponseEvent::ReasoningContentDelta { + delta, + content_index, + }))); } else { continue; } } - Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryDelta(_)))) => { + Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryDelta { .. }))) => { continue; } - Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryPartAdded))) => { + Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryPartAdded { .. }))) => { continue; } Poll::Ready(Some(Ok(ResponseEvent::OutputItemAdded(item)))) => { diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index d1ade21825..98775e3d3a 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -560,6 +560,8 @@ struct SseEvent { response: Option, item: Option, delta: Option, + summary_index: Option, + content_index: Option, } #[derive(Debug, Deserialize)] @@ -819,16 +821,22 @@ async fn process_sse( } } "response.reasoning_summary_text.delta" => { - if let Some(delta) = event.delta { - let event = ResponseEvent::ReasoningSummaryDelta(delta); + if let (Some(delta), Some(summary_index)) = (event.delta, event.summary_index) { + let event = ResponseEvent::ReasoningSummaryDelta { + delta, + summary_index, + }; if tx_event.send(Ok(event)).await.is_err() { return; } } } "response.reasoning_text.delta" => { - if let Some(delta) = event.delta { - let event = ResponseEvent::ReasoningContentDelta(delta); + if let (Some(delta), Some(content_index)) = (event.delta, event.content_index) { + let event = ResponseEvent::ReasoningContentDelta { + delta, + content_index, + }; if tx_event.send(Ok(event)).await.is_err() { return; } @@ -905,10 +913,12 @@ async fn process_sse( } } "response.reasoning_summary_part.added" => { - // Boundary between reasoning summary sections (e.g., titles). - let event = ResponseEvent::ReasoningSummaryPartAdded; - if tx_event.send(Ok(event)).await.is_err() { - return; + if let Some(summary_index) = event.summary_index { + // Boundary between reasoning summary sections (e.g., titles). + let event = ResponseEvent::ReasoningSummaryPartAdded { summary_index }; + if tx_event.send(Ok(event)).await.is_err() { + return; + } } } "response.reasoning_summary_text.done" => {} diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index 7d33751e26..a628e0d320 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -203,9 +203,17 @@ pub enum ResponseEvent { token_usage: Option, }, OutputTextDelta(String), - ReasoningSummaryDelta(String), - ReasoningContentDelta(String), - ReasoningSummaryPartAdded, + ReasoningSummaryDelta { + delta: String, + summary_index: i64, + }, + ReasoningContentDelta { + delta: String, + content_index: i64, + }, + ReasoningSummaryPartAdded { + summary_index: i64, + }, RateLimits(RateLimitSnapshot), } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index bc59c01353..5e9ec5bf1e 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2233,13 +2233,17 @@ async fn try_run_turn( error_or_panic("ReasoningSummaryDelta without active item".to_string()); } } - ResponseEvent::ReasoningSummaryDelta(delta) => { + ResponseEvent::ReasoningSummaryDelta { + delta, + summary_index, + } => { if let Some(active) = active_item.as_ref() { let event = ReasoningContentDeltaEvent { thread_id: sess.conversation_id.to_string(), turn_id: turn_context.sub_id.clone(), item_id: active.id(), - delta: delta.clone(), + delta, + summary_index, }; sess.send_event(&turn_context, EventMsg::ReasoningContentDelta(event)) .await; @@ -2247,18 +2251,29 @@ async fn try_run_turn( error_or_panic("ReasoningSummaryDelta without active item".to_string()); } } - ResponseEvent::ReasoningSummaryPartAdded => { - let event = - EventMsg::AgentReasoningSectionBreak(AgentReasoningSectionBreakEvent {}); - sess.send_event(&turn_context, event).await; + ResponseEvent::ReasoningSummaryPartAdded { summary_index } => { + if let Some(active) = active_item.as_ref() { + let event = + EventMsg::AgentReasoningSectionBreak(AgentReasoningSectionBreakEvent { + item_id: active.id(), + summary_index, + }); + sess.send_event(&turn_context, event).await; + } else { + error_or_panic("ReasoningSummaryPartAdded without active item".to_string()); + } } - ResponseEvent::ReasoningContentDelta(delta) => { + ResponseEvent::ReasoningContentDelta { + delta, + content_index, + } => { if let Some(active) = active_item.as_ref() { let event = ReasoningRawContentDeltaEvent { thread_id: sess.conversation_id.to_string(), turn_id: turn_context.sub_id.clone(), item_id: active.id(), - delta: delta.clone(), + delta, + content_index, }; sess.send_event(&turn_context, EventMsg::ReasoningRawContentDelta(event)) .await; diff --git a/codex-rs/core/tests/chat_completions_sse.rs b/codex-rs/core/tests/chat_completions_sse.rs index 46378b0823..b499b76e8f 100644 --- a/codex-rs/core/tests/chat_completions_sse.rs +++ b/codex-rs/core/tests/chat_completions_sse.rs @@ -215,7 +215,13 @@ async fn streams_reasoning_from_string_delta() { } match &events[1] { - ResponseEvent::ReasoningContentDelta(text) => assert_eq!(text, "think1"), + ResponseEvent::ReasoningContentDelta { + delta, + content_index, + } => { + assert_eq!(delta, "think1"); + assert_eq!(content_index, &0); + } other => panic!("expected reasoning delta, got {other:?}"), } @@ -267,12 +273,24 @@ async fn streams_reasoning_from_object_delta() { } match &events[1] { - ResponseEvent::ReasoningContentDelta(text) => assert_eq!(text, "partA"), + ResponseEvent::ReasoningContentDelta { + delta, + content_index, + } => { + assert_eq!(delta, "partA"); + assert_eq!(content_index, &0); + } other => panic!("expected reasoning delta, got {other:?}"), } match &events[2] { - ResponseEvent::ReasoningContentDelta(text) => assert_eq!(text, "partB"), + ResponseEvent::ReasoningContentDelta { + delta, + content_index, + } => { + assert_eq!(delta, "partB"); + assert_eq!(content_index, &1); + } other => panic!("expected reasoning delta, got {other:?}"), } @@ -319,7 +337,13 @@ async fn streams_reasoning_from_final_message() { } match &events[1] { - ResponseEvent::ReasoningContentDelta(text) => assert_eq!(text, "final-cot"), + ResponseEvent::ReasoningContentDelta { + delta, + content_index, + } => { + assert_eq!(delta, "final-cot"); + assert_eq!(content_index, &0); + } other => panic!("expected reasoning delta, got {other:?}"), } @@ -354,7 +378,13 @@ async fn streams_reasoning_before_tool_call() { } match &events[1] { - ResponseEvent::ReasoningContentDelta(text) => assert_eq!(text, "pre-tool"), + ResponseEvent::ReasoningContentDelta { + delta, + content_index, + } => { + assert_eq!(delta, "pre-tool"); + assert_eq!(content_index, &0); + } other => panic!("expected reasoning delta, got {other:?}"), } diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index 8a62903600..b78f921bc0 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -12,6 +12,8 @@ use wiremock::ResponseTemplate; use wiremock::matchers::method; use wiremock::matchers::path_regex; +use crate::test_codex::ApplyPatchModelOutput; + #[derive(Debug, Clone)] pub struct ResponseMock { requests: Arc>>, @@ -294,6 +296,7 @@ pub fn ev_reasoning_summary_text_delta(delta: &str) -> Value { serde_json::json!({ "type": "response.reasoning_summary_text.delta", "delta": delta, + "summary_index": 0, }) } @@ -301,6 +304,7 @@ pub fn ev_reasoning_text_delta(delta: &str) -> Value { serde_json::json!({ "type": "response.reasoning_text.delta", "delta": delta, + "content_index": 0, }) } @@ -367,6 +371,21 @@ pub fn ev_local_shell_call(call_id: &str, status: &str, command: Vec<&str>) -> V }) } +pub fn ev_apply_patch_call( + call_id: &str, + patch: &str, + output_type: ApplyPatchModelOutput, +) -> Value { + match output_type { + ApplyPatchModelOutput::Freeform => ev_apply_patch_custom_tool_call(call_id, patch), + ApplyPatchModelOutput::Function => ev_apply_patch_function_call(call_id, patch), + ApplyPatchModelOutput::Shell => ev_apply_patch_shell_call(call_id, patch), + ApplyPatchModelOutput::ShellViaHeredoc => { + ev_apply_patch_shell_call_via_heredoc(call_id, patch) + } + } +} + /// Convenience: SSE event for an `apply_patch` custom tool call with raw patch /// text. This mirrors the payload produced by the Responses API when the model /// invokes `apply_patch` directly (before we convert it to a function call). @@ -400,6 +419,21 @@ pub fn ev_apply_patch_function_call(call_id: &str, patch: &str) -> Value { }) } +pub fn ev_apply_patch_shell_call(call_id: &str, patch: &str) -> Value { + let args = serde_json::json!({ "command": ["apply_patch", patch] }); + let arguments = serde_json::to_string(&args).expect("serialize apply_patch arguments"); + + ev_function_call(call_id, "shell", &arguments) +} + +pub fn ev_apply_patch_shell_call_via_heredoc(call_id: &str, patch: &str) -> Value { + let script = format!("apply_patch <<'EOF'\n{patch}\nEOF\n"); + let args = serde_json::json!({ "command": ["bash", "-lc", script] }); + let arguments = serde_json::to_string(&args).expect("serialize apply_patch arguments"); + + ev_function_call(call_id, "shell", &arguments) +} + pub fn sse_failed(id: &str, code: &str, message: &str) -> String { sse(vec![serde_json::json!({ "type": "response.failed", diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index e56446c5ca..2741d8bed1 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -29,6 +29,15 @@ use crate::wait_for_event; type ConfigMutator = dyn FnOnce(&mut Config) + Send; +/// A collection of different ways the model can output an apply_patch call +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub enum ApplyPatchModelOutput { + Freeform, + Function, + Shell, + ShellViaHeredoc, +} + pub struct TestCodexBuilder { config_mutators: Vec>, } @@ -265,6 +274,19 @@ impl TestCodexHarness { .expect("output string") .to_string() } + + pub async fn apply_patch_output( + &self, + call_id: &str, + output_type: ApplyPatchModelOutput, + ) -> String { + match output_type { + ApplyPatchModelOutput::Freeform => self.custom_tool_call_output(call_id).await, + ApplyPatchModelOutput::Function + | ApplyPatchModelOutput::Shell + | ApplyPatchModelOutput::ShellViaHeredoc => self.function_call_stdout(call_id).await, + } + } } fn custom_tool_call_output<'a>(bodies: &'a [Value], call_id: &str) -> &'a Value { diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index 6e9761ae4a..db20370370 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -1,6 +1,8 @@ #![allow(clippy::expect_used)] use anyhow::Result; +use core_test_support::responses::ev_apply_patch_call; +use core_test_support::test_codex::ApplyPatchModelOutput; use pretty_assertions::assert_eq; use std::fs; @@ -25,6 +27,7 @@ use core_test_support::skip_if_no_network; use core_test_support::test_codex::TestCodexHarness; use core_test_support::wait_for_event; use serde_json::json; +use test_case::test_case; async fn apply_patch_harness() -> Result { apply_patch_harness_with(|_| {}).await @@ -45,19 +48,25 @@ async fn mount_apply_patch( call_id: &str, patch: &str, assistant_msg: &str, + output_type: ApplyPatchModelOutput, ) { mount_sse_sequence( harness.server(), - apply_patch_responses(call_id, patch, assistant_msg), + apply_patch_responses(call_id, patch, assistant_msg, output_type), ) .await; } -fn apply_patch_responses(call_id: &str, patch: &str, assistant_msg: &str) -> Vec { +fn apply_patch_responses( + call_id: &str, + patch: &str, + assistant_msg: &str, + output_type: ApplyPatchModelOutput, +) -> Vec { vec![ sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call_id, patch), + ev_apply_patch_call(call_id, patch, output_type), ev_completed("resp-1"), ]), sse(vec![ @@ -68,7 +77,13 @@ fn apply_patch_responses(call_id: &str, patch: &str, assistant_msg: &str) -> Vec } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_multiple_operations_integration() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_multiple_operations_integration( + output_type: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness_with(|config| { @@ -86,11 +101,11 @@ async fn apply_patch_cli_multiple_operations_integration() -> Result<()> { let patch = "*** Begin Patch\n*** Add File: nested/new.txt\n+created\n*** Delete File: delete.txt\n*** Update File: modify.txt\n@@\n-line2\n+changed\n*** End Patch"; let call_id = "apply-multi-ops"; - mount_apply_patch(&harness, call_id, patch, "done").await; + mount_apply_patch(&harness, call_id, patch, "done", output_type).await; harness.submit("please apply multi-ops patch").await?; - let out = harness.function_call_stdout(call_id).await; + let out = harness.apply_patch_output(call_id, output_type).await; let expected = r"(?s)^Exit code: 0 Wall time: [0-9]+(?:\.[0-9]+)? seconds @@ -113,7 +128,11 @@ D delete.txt } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_multiple_chunks() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_multiple_chunks(model_output: ApplyPatchModelOutput) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -123,7 +142,7 @@ async fn apply_patch_cli_multiple_chunks() -> Result<()> { let patch = "*** Begin Patch\n*** Update File: multi.txt\n@@\n-line2\n+changed2\n@@\n-line4\n+changed4\n*** End Patch"; let call_id = "apply-multi-chunks"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; harness.submit("apply multi-chunk patch").await?; @@ -135,7 +154,13 @@ async fn apply_patch_cli_multiple_chunks() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_moves_file_to_new_directory() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_moves_file_to_new_directory( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -147,7 +172,7 @@ async fn apply_patch_cli_moves_file_to_new_directory() -> Result<()> { let patch = "*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/dir/name.txt\n@@\n-old content\n+new content\n*** End Patch"; let call_id = "apply-move"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; harness.submit("apply move patch").await?; @@ -157,7 +182,13 @@ async fn apply_patch_cli_moves_file_to_new_directory() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_updates_file_appends_trailing_newline() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_updates_file_appends_trailing_newline( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -167,7 +198,7 @@ async fn apply_patch_cli_updates_file_appends_trailing_newline() -> Result<()> { let patch = "*** Begin Patch\n*** Update File: no_newline.txt\n@@\n-no newline at end\n+first line\n+second line\n*** End Patch"; let call_id = "apply-append-nl"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; harness.submit("apply newline patch").await?; @@ -178,7 +209,13 @@ async fn apply_patch_cli_updates_file_appends_trailing_newline() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_insert_only_hunk_modifies_file() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_insert_only_hunk_modifies_file( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -188,7 +225,7 @@ async fn apply_patch_cli_insert_only_hunk_modifies_file() -> Result<()> { let patch = "*** Begin Patch\n*** Update File: insert_only.txt\n@@\n alpha\n+beta\n omega\n*** End Patch"; let call_id = "apply-insert-only"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; harness.submit("insert lines via apply_patch").await?; @@ -197,7 +234,13 @@ async fn apply_patch_cli_insert_only_hunk_modifies_file() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_move_overwrites_existing_destination() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_move_overwrites_existing_destination( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -211,7 +254,7 @@ async fn apply_patch_cli_move_overwrites_existing_destination() -> Result<()> { let patch = "*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/dir/name.txt\n@@\n-from\n+new\n*** End Patch"; let call_id = "apply-move-overwrite"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; harness.submit("apply move overwrite patch").await?; @@ -221,7 +264,13 @@ async fn apply_patch_cli_move_overwrites_existing_destination() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_move_without_content_change_has_no_turn_diff() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_move_without_content_change_has_no_turn_diff( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -236,7 +285,7 @@ async fn apply_patch_cli_move_without_content_change_has_no_turn_diff() -> Resul let patch = "*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/name.txt\n@@\n same\n*** End Patch"; let call_id = "apply-move-no-change"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; let model = test.session_configured.model.clone(); codex @@ -272,7 +321,13 @@ async fn apply_patch_cli_move_without_content_change_has_no_turn_diff() -> Resul } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_add_overwrites_existing_file() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_add_overwrites_existing_file( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -282,7 +337,7 @@ async fn apply_patch_cli_add_overwrites_existing_file() -> Result<()> { let patch = "*** Begin Patch\n*** Add File: duplicate.txt\n+new content\n*** End Patch"; let call_id = "apply-add-overwrite"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; harness.submit("apply add overwrite patch").await?; @@ -291,18 +346,24 @@ async fn apply_patch_cli_add_overwrites_existing_file() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_rejects_invalid_hunk_header() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_rejects_invalid_hunk_header( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; let patch = "*** Begin Patch\n*** Frobnicate File: foo\n*** End Patch"; let call_id = "apply-invalid-header"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; harness.submit("apply invalid header patch").await?; - let out = harness.function_call_stdout(call_id).await; + let out = harness.apply_patch_output(call_id, model_output).await; assert!( out.contains("apply_patch verification failed"), @@ -316,7 +377,13 @@ async fn apply_patch_cli_rejects_invalid_hunk_header() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_reports_missing_context() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_reports_missing_context( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -327,11 +394,11 @@ async fn apply_patch_cli_reports_missing_context() -> Result<()> { let patch = "*** Begin Patch\n*** Update File: modify.txt\n@@\n-missing\n+changed\n*** End Patch"; let call_id = "apply-missing-context"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; harness.submit("apply missing context patch").await?; - let out = harness.function_call_stdout(call_id).await; + let out = harness.apply_patch_output(call_id, model_output).await; assert!( out.contains("apply_patch verification failed"), @@ -343,18 +410,24 @@ async fn apply_patch_cli_reports_missing_context() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_reports_missing_target_file() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_reports_missing_target_file( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; let patch = "*** Begin Patch\n*** Update File: missing.txt\n@@\n-nope\n+better\n*** End Patch"; let call_id = "apply-missing-file"; - mount_apply_patch(&harness, call_id, patch, "fail").await; + mount_apply_patch(&harness, call_id, patch, "fail", model_output).await; harness.submit("attempt to update a missing file").await?; - let out = harness.function_call_stdout(call_id).await; + let out = harness.apply_patch_output(call_id, model_output).await; assert!( out.contains("apply_patch verification failed"), "expected verification failure message" @@ -372,18 +445,24 @@ async fn apply_patch_cli_reports_missing_target_file() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_delete_missing_file_reports_error() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_delete_missing_file_reports_error( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; let patch = "*** Begin Patch\n*** Delete File: missing.txt\n*** End Patch"; let call_id = "apply-delete-missing"; - mount_apply_patch(&harness, call_id, patch, "fail").await; + mount_apply_patch(&harness, call_id, patch, "fail", model_output).await; harness.submit("attempt to delete missing file").await?; - let out = harness.function_call_stdout(call_id).await; + let out = harness.apply_patch_output(call_id, model_output).await; assert!( out.contains("apply_patch verification failed"), @@ -402,18 +481,22 @@ async fn apply_patch_cli_delete_missing_file_reports_error() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_rejects_empty_patch() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_rejects_empty_patch(model_output: ApplyPatchModelOutput) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; let patch = "*** Begin Patch\n*** End Patch"; let call_id = "apply-empty"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; harness.submit("apply empty patch").await?; - let out = harness.function_call_stdout(call_id).await; + let out = harness.apply_patch_output(call_id, model_output).await; assert!( out.contains("patch rejected: empty patch"), "expected rejection for empty patch: {out}" @@ -422,7 +505,13 @@ async fn apply_patch_cli_rejects_empty_patch() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_delete_directory_reports_verification_error() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_delete_directory_reports_verification_error( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -431,18 +520,24 @@ async fn apply_patch_cli_delete_directory_reports_verification_error() -> Result let patch = "*** Begin Patch\n*** Delete File: dir\n*** End Patch"; let call_id = "apply-delete-dir"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; harness.submit("delete a directory via apply_patch").await?; - let out = harness.function_call_stdout(call_id).await; + let out = harness.apply_patch_output(call_id, model_output).await; assert!(out.contains("apply_patch verification failed")); assert!(out.contains("Failed to read")); Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_rejects_path_traversal_outside_workspace() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_rejects_path_traversal_outside_workspace( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -458,7 +553,7 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace() -> Result<() let patch = "*** Begin Patch\n*** Add File: ../escape.txt\n+outside\n*** End Patch"; let call_id = "apply-path-traversal"; - mount_apply_patch(&harness, call_id, patch, "fail").await; + mount_apply_patch(&harness, call_id, patch, "fail", model_output).await; let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], @@ -473,7 +568,7 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace() -> Result<() ) .await?; - let out = harness.function_call_stdout(call_id).await; + let out = harness.apply_patch_output(call_id, model_output).await; assert!( out.contains( "patch rejected: writing outside of the project; rejected by user approval settings" @@ -488,7 +583,13 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace() -> Result<() } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -507,7 +608,7 @@ async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace() -> Resu let patch = "*** Begin Patch\n*** Update File: stay.txt\n*** Move to: ../escape-move.txt\n@@\n-from\n+to\n*** End Patch"; let call_id = "apply-move-traversal"; - mount_apply_patch(&harness, call_id, patch, "fail").await; + mount_apply_patch(&harness, call_id, patch, "fail", model_output).await; let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], @@ -519,7 +620,7 @@ async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace() -> Resu .submit_with_policy("attempt move traversal via apply_patch", sandbox_policy) .await?; - let out = harness.function_call_stdout(call_id).await; + let out = harness.apply_patch_output(call_id, model_output).await; assert!( out.contains( "patch rejected: writing outside of the project; rejected by user approval settings" @@ -535,7 +636,13 @@ async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace() -> Resu } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_verification_failure_has_no_side_effects() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_verification_failure_has_no_side_effects( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness_with(|config| { @@ -547,7 +654,7 @@ async fn apply_patch_cli_verification_failure_has_no_side_effects() -> Result<() let call_id = "apply-partial-no-side-effects"; let patch = "*** Begin Patch\n*** Add File: created.txt\n+hello\n*** Update File: missing.txt\n@@\n-old\n+new\n*** End Patch"; - mount_apply_patch(&harness, call_id, patch, "failed").await; + mount_apply_patch(&harness, call_id, patch, "failed", model_output).await; harness.submit("attempt partial apply patch").await?; @@ -700,7 +807,14 @@ async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch() -> Result< format!("*** Begin Patch\n*** Add File: {file_name}\n+lenient\n*** End Patch\n"); let wrapped = format!("<<'EOF'\n{patch_inner}EOF\n"); let call_id = "apply-lenient"; - mount_apply_patch(&harness, call_id, &wrapped, "ok").await; + mount_apply_patch( + &harness, + call_id, + wrapped.as_str(), + "ok", + ApplyPatchModelOutput::Function, + ) + .await; harness.submit("apply lenient heredoc patch").await?; @@ -710,7 +824,11 @@ async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch() -> Result< } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_end_of_file_anchor() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_end_of_file_anchor(model_output: ApplyPatchModelOutput) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -720,7 +838,7 @@ async fn apply_patch_cli_end_of_file_anchor() -> Result<()> { let patch = "*** Begin Patch\n*** Update File: tail.txt\n@@\n-last\n+end\n*** End of File\n*** End Patch"; let call_id = "apply-eof"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; harness.submit("apply EOF-anchored patch").await?; assert_eq!(fs::read_to_string(&target)?, "alpha\nend\n"); @@ -728,7 +846,13 @@ async fn apply_patch_cli_end_of_file_anchor() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_missing_second_chunk_context_rejected() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_cli_missing_second_chunk_context_rejected( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -740,11 +864,11 @@ async fn apply_patch_cli_missing_second_chunk_context_rejected() -> Result<()> { let patch = "*** Begin Patch\n*** Update File: two_chunks.txt\n@@\n-b\n+B\n\n-d\n+D\n*** End Patch"; let call_id = "apply-missing-ctx-2nd"; - mount_apply_patch(&harness, call_id, patch, "fail").await; + mount_apply_patch(&harness, call_id, patch, "fail", model_output).await; harness.submit("apply missing context second chunk").await?; - let out = harness.function_call_stdout(call_id).await; + let out = harness.apply_patch_output(call_id, model_output).await; assert!(out.contains("apply_patch verification failed")); assert!( out.contains("Failed to find expected lines in"), @@ -756,7 +880,13 @@ async fn apply_patch_cli_missing_second_chunk_context_rejected() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_emits_turn_diff_event_with_unified_diff() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_emits_turn_diff_event_with_unified_diff( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -767,16 +897,7 @@ async fn apply_patch_emits_turn_diff_event_with_unified_diff() -> Result<()> { let call_id = "apply-diff-event"; let file = "udiff.txt"; let patch = format!("*** Begin Patch\n*** Add File: {file}\n+hello\n*** End Patch\n"); - let first = sse(vec![ - ev_response_created("resp-1"), - ev_apply_patch_function_call(call_id, &patch), - ev_completed("resp-1"), - ]); - let second = sse(vec![ - ev_assistant_message("msg-1", "ok"), - ev_completed("resp-2"), - ]); - mount_sse_sequence(harness.server(), vec![first, second]).await; + mount_apply_patch(&harness, call_id, patch.as_str(), "ok", model_output).await; let model = test.session_configured.model.clone(); codex @@ -814,7 +935,13 @@ async fn apply_patch_emits_turn_diff_event_with_unified_diff() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_turn_diff_for_rename_with_content_change() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_turn_diff_for_rename_with_content_change( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -829,16 +956,7 @@ async fn apply_patch_turn_diff_for_rename_with_content_change() -> Result<()> { // Patch: update + move let call_id = "apply-rename-change"; let patch = "*** Begin Patch\n*** Update File: old.txt\n*** Move to: new.txt\n@@\n-old\n+new\n*** End Patch"; - let first = sse(vec![ - ev_response_created("resp-1"), - ev_apply_patch_function_call(call_id, patch), - ev_completed("resp-1"), - ]); - let second = sse(vec![ - ev_assistant_message("msg-1", "ok"), - ev_completed("resp-2"), - ]); - mount_sse_sequence(harness.server(), vec![first, second]).await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; let model = test.session_configured.model.clone(); codex @@ -1031,7 +1149,13 @@ async fn apply_patch_aggregates_diff_preserves_success_after_failure() -> Result } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_change_context_disambiguates_target() -> Result<()> { +#[test_case(ApplyPatchModelOutput::Freeform)] +#[test_case(ApplyPatchModelOutput::Function)] +#[test_case(ApplyPatchModelOutput::Shell)] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] +async fn apply_patch_change_context_disambiguates_target( + model_output: ApplyPatchModelOutput, +) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; @@ -1042,7 +1166,7 @@ async fn apply_patch_change_context_disambiguates_target() -> Result<()> { let patch = "*** Begin Patch\n*** Update File: multi_ctx.txt\n@@ fn b\n-x=10\n+x=11\n*** End Patch"; let call_id = "apply-ctx"; - mount_apply_patch(&harness, call_id, patch, "ok").await; + mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; harness.submit("apply with change_context").await?; diff --git a/codex-rs/core/tests/suite/apply_patch_freeform.rs b/codex-rs/core/tests/suite/apply_patch_freeform.rs deleted file mode 100644 index 72c405eb1b..0000000000 --- a/codex-rs/core/tests/suite/apply_patch_freeform.rs +++ /dev/null @@ -1,1001 +0,0 @@ -#![allow(clippy::expect_used)] - -use anyhow::Result; -use core_test_support::responses::ev_apply_patch_custom_tool_call; -use pretty_assertions::assert_eq; -use std::fs; - -use codex_core::config::Config; -use codex_core::features::Feature; -use codex_core::model_family::find_family_for_model; -use codex_core::protocol::AskForApproval; -use codex_core::protocol::EventMsg; -use codex_core::protocol::Op; -use codex_core::protocol::SandboxPolicy; -use codex_protocol::config_types::ReasoningSummary; -use codex_protocol::user_input::UserInput; -use core_test_support::assert_regex_match; -use core_test_support::responses::ev_assistant_message; -use core_test_support::responses::ev_completed; -use core_test_support::responses::ev_response_created; -use core_test_support::responses::mount_sse_sequence; -use core_test_support::responses::sse; -use core_test_support::skip_if_no_network; -use core_test_support::test_codex::TestCodexHarness; -use core_test_support::wait_for_event; - -async fn apply_patch_harness() -> Result { - apply_patch_harness_with(|_| {}).await -} - -async fn apply_patch_harness_with( - configure: impl FnOnce(&mut Config) + Send + 'static, -) -> Result { - TestCodexHarness::with_config(|config| { - config.include_apply_patch_tool = true; - configure(config); - }) - .await -} - -async fn mount_apply_patch( - harness: &TestCodexHarness, - call_id: &str, - patch: &str, - assistant_msg: &str, -) { - mount_sse_sequence( - harness.server(), - freeform_apply_patch_responses(call_id, patch, assistant_msg), - ) - .await; -} - -fn freeform_apply_patch_responses(call_id: &str, patch: &str, assistant_msg: &str) -> Vec { - vec![ - sse(vec![ - ev_response_created("resp-1"), - ev_apply_patch_custom_tool_call(call_id, patch), - ev_completed("resp-1"), - ]), - sse(vec![ - ev_assistant_message("msg-1", assistant_msg), - ev_completed("resp-2"), - ]), - ] -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_multiple_operations_integration() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness_with(|config| { - config.model = "gpt-5".to_string(); - config.model_family = find_family_for_model("gpt-5").expect("gpt-5 is valid"); - }) - .await?; - - // Seed workspace state - let modify_path = harness.path("modify.txt"); - let delete_path = harness.path("delete.txt"); - fs::write(&modify_path, "line1\nline2\n")?; - fs::write(&delete_path, "obsolete\n")?; - - let patch = "*** Begin Patch\n*** Add File: nested/new.txt\n+created\n*** Delete File: delete.txt\n*** Update File: modify.txt\n@@\n-line2\n+changed\n*** End Patch"; - - let call_id = "apply-multi-ops"; - mount_apply_patch(&harness, call_id, patch, "done").await; - - harness.submit("please apply multi-ops patch").await?; - - let out = harness.custom_tool_call_output(call_id).await; - - let expected = r"(?s)^Exit code: 0 -Wall time: [0-9]+(?:\.[0-9]+)? seconds -Output: -Success. Updated the following files: -A nested/new.txt -M modify.txt -D delete.txt -?$"; - assert_regex_match(expected, &out); - - assert_eq!( - fs::read_to_string(harness.path("nested/new.txt"))?, - "created\n" - ); - assert_eq!(fs::read_to_string(&modify_path)?, "line1\nchanged\n"); - assert!(!delete_path.exists()); - - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_multiple_chunks() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let target = harness.path("multi.txt"); - fs::write(&target, "line1\nline2\nline3\nline4\n")?; - - let patch = "*** Begin Patch\n*** Update File: multi.txt\n@@\n-line2\n+changed2\n@@\n-line4\n+changed4\n*** End Patch"; - let call_id = "apply-multi-chunks"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - harness.submit("apply multi-chunk patch").await?; - - assert_eq!( - fs::read_to_string(&target)?, - "line1\nchanged2\nline3\nchanged4\n" - ); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_moves_file_to_new_directory() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let original = harness.path("old/name.txt"); - let new_path = harness.path("renamed/dir/name.txt"); - fs::create_dir_all(original.parent().expect("parent"))?; - fs::write(&original, "old content\n")?; - - let patch = "*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/dir/name.txt\n@@\n-old content\n+new content\n*** End Patch"; - let call_id = "apply-move"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - harness.submit("apply move patch").await?; - - assert!(!original.exists()); - assert_eq!(fs::read_to_string(&new_path)?, "new content\n"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_updates_file_appends_trailing_newline() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let target = harness.path("no_newline.txt"); - fs::write(&target, "no newline at end")?; - - let patch = "*** Begin Patch\n*** Update File: no_newline.txt\n@@\n-no newline at end\n+first line\n+second line\n*** End Patch"; - let call_id = "apply-append-nl"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - harness.submit("apply newline patch").await?; - - let contents = fs::read_to_string(&target)?; - assert!(contents.ends_with('\n')); - assert_eq!(contents, "first line\nsecond line\n"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_insert_only_hunk_modifies_file() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let target = harness.path("insert_only.txt"); - fs::write(&target, "alpha\nomega\n")?; - - let patch = "*** Begin Patch\n*** Update File: insert_only.txt\n@@\n alpha\n+beta\n omega\n*** End Patch"; - let call_id = "apply-insert-only"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - harness.submit("insert lines via apply_patch").await?; - - assert_eq!(fs::read_to_string(&target)?, "alpha\nbeta\nomega\n"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_move_overwrites_existing_destination() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let original = harness.path("old/name.txt"); - let destination = harness.path("renamed/dir/name.txt"); - fs::create_dir_all(original.parent().expect("parent"))?; - fs::create_dir_all(destination.parent().expect("parent"))?; - fs::write(&original, "from\n")?; - fs::write(&destination, "existing\n")?; - - let patch = "*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/dir/name.txt\n@@\n-from\n+new\n*** End Patch"; - let call_id = "apply-move-overwrite"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - harness.submit("apply move overwrite patch").await?; - - assert!(!original.exists()); - assert_eq!(fs::read_to_string(&destination)?, "new\n"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_move_without_content_change_has_no_turn_diff() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - let test = harness.test(); - let codex = test.codex.clone(); - let cwd = test.cwd.clone(); - - let original = harness.path("old/name.txt"); - let destination = harness.path("renamed/name.txt"); - fs::create_dir_all(original.parent().expect("parent should exist"))?; - fs::write(&original, "same\n")?; - - let patch = "*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/name.txt\n@@\n same\n*** End Patch"; - let call_id = "apply-move-no-change"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - items: vec![UserInput::Text { - text: "rename without content change".into(), - }], - final_output_json_schema: None, - cwd: cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::DangerFullAccess, - model, - effort: None, - summary: ReasoningSummary::Auto, - }) - .await?; - - let mut saw_turn_diff = false; - wait_for_event(&codex, |event| match event { - EventMsg::TurnDiff(_) => { - saw_turn_diff = true; - false - } - EventMsg::TaskComplete(_) => true, - _ => false, - }) - .await; - - assert!(!saw_turn_diff, "pure rename should not emit a turn diff"); - assert!(!original.exists()); - assert_eq!(fs::read_to_string(&destination)?, "same\n"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_add_overwrites_existing_file() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let path = harness.path("duplicate.txt"); - fs::write(&path, "old content\n")?; - - let patch = "*** Begin Patch\n*** Add File: duplicate.txt\n+new content\n*** End Patch"; - let call_id = "apply-add-overwrite"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - harness.submit("apply add overwrite patch").await?; - - assert_eq!(fs::read_to_string(&path)?, "new content\n"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_rejects_invalid_hunk_header() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let patch = "*** Begin Patch\n*** Frobnicate File: foo\n*** End Patch"; - let call_id = "apply-invalid-header"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - harness.submit("apply invalid header patch").await?; - - let out = harness.custom_tool_call_output(call_id).await; - - assert!( - out.contains("apply_patch verification failed"), - "expected verification failure message" - ); - assert!( - out.contains("is not a valid hunk header"), - "expected parse diagnostics in output: {out:?}" - ); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_reports_missing_context() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let target = harness.path("modify.txt"); - fs::write(&target, "line1\nline2\n")?; - - let patch = - "*** Begin Patch\n*** Update File: modify.txt\n@@\n-missing\n+changed\n*** End Patch"; - let call_id = "apply-missing-context"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - harness.submit("apply missing context patch").await?; - - let out = harness.custom_tool_call_output(call_id).await; - - assert!( - out.contains("apply_patch verification failed"), - "expected verification failure message" - ); - assert!(out.contains("Failed to find expected lines in")); - assert_eq!(fs::read_to_string(&target)?, "line1\nline2\n"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_reports_missing_target_file() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let patch = "*** Begin Patch\n*** Update File: missing.txt\n@@\n-nope\n+better\n*** End Patch"; - let call_id = "apply-missing-file"; - mount_apply_patch(&harness, call_id, patch, "fail").await; - - harness.submit("attempt to update a missing file").await?; - - let out = harness.custom_tool_call_output(call_id).await; - assert!( - out.contains("apply_patch verification failed"), - "expected verification failure message" - ); - assert!( - out.contains("Failed to read file to update"), - "expected missing file diagnostics: {out}" - ); - assert!( - out.contains("missing.txt"), - "expected missing file path in diagnostics: {out}" - ); - assert!(!harness.path("missing.txt").exists()); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_delete_missing_file_reports_error() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let patch = "*** Begin Patch\n*** Delete File: missing.txt\n*** End Patch"; - let call_id = "apply-delete-missing"; - mount_apply_patch(&harness, call_id, patch, "fail").await; - - harness.submit("attempt to delete missing file").await?; - - let out = harness.custom_tool_call_output(call_id).await; - - assert!( - out.contains("apply_patch verification failed"), - "expected verification failure message: {out}" - ); - assert!( - out.contains("Failed to read"), - "missing delete diagnostics should mention read failure: {out}" - ); - assert!( - out.contains("missing.txt"), - "missing delete diagnostics should surface target path: {out}" - ); - assert!(!harness.path("missing.txt").exists()); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_rejects_empty_patch() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let patch = "*** Begin Patch\n*** End Patch"; - let call_id = "apply-empty"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - harness.submit("apply empty patch").await?; - - let out = harness.custom_tool_call_output(call_id).await; - assert!( - out.contains("patch rejected: empty patch"), - "expected rejection for empty patch: {out}" - ); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_delete_directory_reports_verification_error() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - fs::create_dir(harness.path("dir"))?; - - let patch = "*** Begin Patch\n*** Delete File: dir\n*** End Patch"; - let call_id = "apply-delete-dir"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - harness.submit("delete a directory via apply_patch").await?; - - let out = harness.custom_tool_call_output(call_id).await; - assert!(out.contains("apply_patch verification failed")); - assert!(out.contains("Failed to read")); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_rejects_path_traversal_outside_workspace() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let escape_path = harness - .test() - .cwd - .path() - .parent() - .expect("cwd should have parent") - .join("escape.txt"); - let _ = fs::remove_file(&escape_path); - - let patch = "*** Begin Patch\n*** Add File: ../escape.txt\n+outside\n*** End Patch"; - let call_id = "apply-path-traversal"; - mount_apply_patch(&harness, call_id, patch, "fail").await; - - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }; - harness - .submit_with_policy( - "attempt to escape workspace via apply_patch", - sandbox_policy, - ) - .await?; - - let out = harness.custom_tool_call_output(call_id).await; - assert!( - out.contains( - "patch rejected: writing outside of the project; rejected by user approval settings" - ), - "expected rejection message for path traversal: {out}" - ); - assert!( - !escape_path.exists(), - "path traversal should be rejected; tool output: {out}" - ); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let escape_path = harness - .test() - .cwd - .path() - .parent() - .expect("cwd should have parent") - .join("escape-move.txt"); - let _ = fs::remove_file(&escape_path); - - let source = harness.path("stay.txt"); - fs::write(&source, "from\n")?; - - let patch = "*** Begin Patch\n*** Update File: stay.txt\n*** Move to: ../escape-move.txt\n@@\n-from\n+to\n*** End Patch"; - let call_id = "apply-move-traversal"; - mount_apply_patch(&harness, call_id, patch, "fail").await; - - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }; - harness - .submit_with_policy("attempt move traversal via apply_patch", sandbox_policy) - .await?; - - let out = harness.custom_tool_call_output(call_id).await; - assert!( - out.contains( - "patch rejected: writing outside of the project; rejected by user approval settings" - ), - "expected rejection message for path traversal: {out}" - ); - assert!( - !escape_path.exists(), - "move path traversal should be rejected; tool output: {out}" - ); - assert_eq!(fs::read_to_string(&source)?, "from\n"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_verification_failure_has_no_side_effects() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness_with(|config| { - config.features.enable(Feature::ApplyPatchFreeform); - }) - .await?; - - // Compose a patch that would create a file, then fail verification on an update. - let call_id = "apply-partial-no-side-effects"; - let patch = "*** Begin Patch\n*** Add File: created.txt\n+hello\n*** Update File: missing.txt\n@@\n-old\n+new\n*** End Patch"; - - mount_apply_patch(&harness, call_id, patch, "failed").await; - - harness.submit("attempt partial apply patch").await?; - - let created = harness.path("created.txt"); - assert!( - !created.exists(), - "verification failure should prevent any filesystem changes" - ); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_shell_failure_propagates_error_and_skips_diff() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness_with(|config| { - config.model = "gpt-5".to_string(); - config.model_family = find_family_for_model("gpt-5").expect("gpt-5 is valid"); - }) - .await?; - let test = harness.test(); - let codex = test.codex.clone(); - let cwd = test.cwd.clone(); - - let target = cwd.path().join("invalid.txt"); - fs::write(&target, "ok\n")?; - - let patch = - "*** Begin Patch\n*** Update File: invalid.txt\n@@\n-nope\n+changed\n*** End Patch\n"; - let call_id = "shell-apply-failure"; - let bodies = vec![ - sse(vec![ - ev_response_created("resp-1"), - ev_apply_patch_custom_tool_call(call_id, patch), - ev_completed("resp-1"), - ]), - sse(vec![ - ev_assistant_message("msg-1", "fail"), - ev_completed("resp-2"), - ]), - ]; - mount_sse_sequence(harness.server(), bodies).await; - - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - items: vec![UserInput::Text { - text: "apply patch via shell".into(), - }], - final_output_json_schema: None, - cwd: cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::DangerFullAccess, - model, - effort: None, - summary: ReasoningSummary::Auto, - }) - .await?; - - let mut saw_turn_diff = false; - wait_for_event(&codex, |event| match event { - EventMsg::TurnDiff(_) => { - saw_turn_diff = true; - false - } - EventMsg::TaskComplete(_) => true, - _ => false, - }) - .await; - - assert!( - !saw_turn_diff, - "turn diff should not be emitted when shell apply_patch fails verification" - ); - - let out = harness.custom_tool_call_output(call_id).await; - assert!( - out.contains("apply_patch verification failed"), - "expected verification failure message" - ); - assert!( - out.contains("Failed to find expected lines in"), - "expected failure diagnostics: {out}" - ); - assert!( - out.contains("invalid.txt"), - "expected file path in output: {out}" - ); - assert_eq!(fs::read_to_string(&target)?, "ok\n"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let file_name = "lenient.txt"; - let patch_inner = - format!("*** Begin Patch\n*** Add File: {file_name}\n+lenient\n*** End Patch\n"); - let wrapped = format!("<<'EOF'\n{patch_inner}EOF\n"); - let call_id = "apply-lenient"; - mount_apply_patch(&harness, call_id, &wrapped, "ok").await; - - harness.submit("apply lenient heredoc patch").await?; - - let new_file = harness.path(file_name); - assert_eq!(fs::read_to_string(new_file)?, "lenient\n"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_end_of_file_anchor() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let target = harness.path("tail.txt"); - fs::write(&target, "alpha\nlast\n")?; - - let patch = "*** Begin Patch\n*** Update File: tail.txt\n@@\n-last\n+end\n*** End of File\n*** End Patch"; - let call_id = "apply-eof"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - harness.submit("apply EOF-anchored patch").await?; - assert_eq!(fs::read_to_string(&target)?, "alpha\nend\n"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_cli_missing_second_chunk_context_rejected() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let target = harness.path("two_chunks.txt"); - fs::write(&target, "a\nb\nc\nd\n")?; - - // First chunk has @@, second chunk intentionally omits @@ to trigger parse error. - let patch = - "*** Begin Patch\n*** Update File: two_chunks.txt\n@@\n-b\n+B\n\n-d\n+D\n*** End Patch"; - let call_id = "apply-missing-ctx-2nd"; - mount_apply_patch(&harness, call_id, patch, "fail").await; - - harness.submit("apply missing context second chunk").await?; - - let out = harness.custom_tool_call_output(call_id).await; - assert!(out.contains("apply_patch verification failed")); - assert!( - out.contains("Failed to find expected lines in"), - "expected hunk context diagnostics: {out}" - ); - // Original file unchanged on failure - assert_eq!(fs::read_to_string(&target)?, "a\nb\nc\nd\n"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_emits_turn_diff_event_with_unified_diff() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - let test = harness.test(); - let codex = test.codex.clone(); - let cwd = test.cwd.clone(); - - let call_id = "apply-diff-event"; - let file = "udiff.txt"; - let patch = format!("*** Begin Patch\n*** Add File: {file}\n+hello\n*** End Patch\n"); - let first = sse(vec![ - ev_response_created("resp-1"), - ev_apply_patch_custom_tool_call(call_id, &patch), - ev_completed("resp-1"), - ]); - let second = sse(vec![ - ev_assistant_message("msg-1", "ok"), - ev_completed("resp-2"), - ]); - mount_sse_sequence(harness.server(), vec![first, second]).await; - - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - items: vec![UserInput::Text { - text: "emit diff".into(), - }], - final_output_json_schema: None, - cwd: cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::DangerFullAccess, - model, - effort: None, - summary: ReasoningSummary::Auto, - }) - .await?; - - let mut saw_turn_diff = None; - wait_for_event(&codex, |event| match event { - EventMsg::TurnDiff(ev) => { - saw_turn_diff = Some(ev.unified_diff.clone()); - false - } - EventMsg::TaskComplete(_) => true, - _ => false, - }) - .await; - - let diff = saw_turn_diff.expect("expected TurnDiff event"); - // Basic markers of a unified diff with file addition - assert!(diff.contains("diff --git"), "diff header missing: {diff:?}"); - assert!(diff.contains("--- /dev/null") || diff.contains("--- a/")); - assert!(diff.contains("+++ b/")); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_turn_diff_for_rename_with_content_change() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - let test = harness.test(); - let codex = test.codex.clone(); - let cwd = test.cwd.clone(); - - // Seed original file - let old = cwd.path().join("old.txt"); - fs::write(&old, "old\n")?; - - // Patch: update + move - let call_id = "apply-rename-change"; - let patch = "*** Begin Patch\n*** Update File: old.txt\n*** Move to: new.txt\n@@\n-old\n+new\n*** End Patch"; - let first = sse(vec![ - ev_response_created("resp-1"), - ev_apply_patch_custom_tool_call(call_id, patch), - ev_completed("resp-1"), - ]); - let second = sse(vec![ - ev_assistant_message("msg-1", "ok"), - ev_completed("resp-2"), - ]); - mount_sse_sequence(harness.server(), vec![first, second]).await; - - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - items: vec![UserInput::Text { - text: "rename with change".into(), - }], - final_output_json_schema: None, - cwd: cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::DangerFullAccess, - model, - effort: None, - summary: ReasoningSummary::Auto, - }) - .await?; - - let mut last_diff: Option = None; - wait_for_event(&codex, |event| match event { - EventMsg::TurnDiff(ev) => { - last_diff = Some(ev.unified_diff.clone()); - false - } - EventMsg::TaskComplete(_) => true, - _ => false, - }) - .await; - - let diff = last_diff.expect("expected TurnDiff event after rename"); - // Basic checks: shows old -> new, and the content delta - assert!(diff.contains("old.txt"), "diff missing old path: {diff:?}"); - assert!(diff.contains("new.txt"), "diff missing new path: {diff:?}"); - assert!(diff.contains("--- a/"), "missing old header"); - assert!(diff.contains("+++ b/"), "missing new header"); - assert!(diff.contains("-old\n"), "missing removal line"); - assert!(diff.contains("+new\n"), "missing addition line"); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_aggregates_diff_across_multiple_tool_calls() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - let test = harness.test(); - let codex = test.codex.clone(); - let cwd = test.cwd.clone(); - - let call1 = "agg-1"; - let call2 = "agg-2"; - let patch1 = "*** Begin Patch\n*** Add File: agg/a.txt\n+v1\n*** End Patch"; - let patch2 = "*** Begin Patch\n*** Update File: agg/a.txt\n@@\n-v1\n+v2\n*** Add File: agg/b.txt\n+B\n*** End Patch"; - - let s1 = sse(vec![ - ev_response_created("resp-1"), - ev_apply_patch_custom_tool_call(call1, patch1), - ev_completed("resp-1"), - ]); - let s2 = sse(vec![ - ev_response_created("resp-2"), - ev_apply_patch_custom_tool_call(call2, patch2), - ev_completed("resp-2"), - ]); - let s3 = sse(vec![ - ev_assistant_message("msg-1", "ok"), - ev_completed("resp-3"), - ]); - mount_sse_sequence(harness.server(), vec![s1, s2, s3]).await; - - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - items: vec![UserInput::Text { - text: "aggregate diffs".into(), - }], - final_output_json_schema: None, - cwd: cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::DangerFullAccess, - model, - effort: None, - summary: ReasoningSummary::Auto, - }) - .await?; - - let mut last_diff: Option = None; - wait_for_event(&codex, |event| match event { - EventMsg::TurnDiff(ev) => { - last_diff = Some(ev.unified_diff.clone()); - false - } - EventMsg::TaskComplete(_) => true, - _ => false, - }) - .await; - - let diff = last_diff.expect("expected TurnDiff after two patches"); - assert!(diff.contains("agg/a.txt"), "diff missing a.txt"); - assert!(diff.contains("agg/b.txt"), "diff missing b.txt"); - // Final content reflects v2 for a.txt - assert!(diff.contains("+v2\n") || diff.contains("v2\n")); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_aggregates_diff_preserves_success_after_failure() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - let test = harness.test(); - let codex = test.codex.clone(); - let cwd = test.cwd.clone(); - - let call_success = "agg-success"; - let call_failure = "agg-failure"; - let patch_success = "*** Begin Patch\n*** Add File: partial/success.txt\n+ok\n*** End Patch"; - let patch_failure = - "*** Begin Patch\n*** Update File: partial/success.txt\n@@\n-missing\n+new\n*** End Patch"; - - let responses = vec![ - sse(vec![ - ev_response_created("resp-1"), - ev_apply_patch_custom_tool_call(call_success, patch_success), - ev_completed("resp-1"), - ]), - sse(vec![ - ev_response_created("resp-2"), - ev_apply_patch_custom_tool_call(call_failure, patch_failure), - ev_completed("resp-2"), - ]), - sse(vec![ - ev_assistant_message("msg-1", "failed"), - ev_completed("resp-3"), - ]), - ]; - mount_sse_sequence(harness.server(), responses).await; - - let model = test.session_configured.model.clone(); - codex - .submit(Op::UserTurn { - items: vec![UserInput::Text { - text: "apply patch twice with failure".into(), - }], - final_output_json_schema: None, - cwd: cwd.path().to_path_buf(), - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::DangerFullAccess, - model, - effort: None, - summary: ReasoningSummary::Auto, - }) - .await?; - - let mut last_diff: Option = None; - wait_for_event(&codex, |event| match event { - EventMsg::TurnDiff(ev) => { - last_diff = Some(ev.unified_diff.clone()); - false - } - EventMsg::TaskComplete(_) => true, - _ => false, - }) - .await; - - let diff = last_diff.expect("expected TurnDiff after failed patch"); - assert!( - diff.contains("partial/success.txt"), - "diff should still include the successful addition: {diff}" - ); - assert!( - diff.contains("+ok"), - "diff should include contents from successful patch: {diff}" - ); - - let failure_out = harness.custom_tool_call_output(call_failure).await; - assert!( - failure_out.contains("apply_patch verification failed"), - "expected verification failure output: {failure_out}" - ); - assert!( - failure_out.contains("Failed to find expected lines in"), - "expected missing context diagnostics: {failure_out}" - ); - - assert_eq!( - fs::read_to_string(cwd.path().join("partial/success.txt"))?, - "ok\n" - ); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn apply_patch_change_context_disambiguates_target() -> Result<()> { - skip_if_no_network!(Ok(())); - - let harness = apply_patch_harness().await?; - - let target = harness.path("multi_ctx.txt"); - fs::write(&target, "fn a\nx=10\ny=2\nfn b\nx=10\ny=20\n")?; - - let patch = - "*** Begin Patch\n*** Update File: multi_ctx.txt\n@@ fn b\n-x=10\n+x=11\n*** End Patch"; - let call_id = "apply-ctx"; - mount_apply_patch(&harness, call_id, patch, "ok").await; - - harness.submit("apply with change_context").await?; - - let contents = fs::read_to_string(&target)?; - assert_eq!(contents, "fn a\nx=10\ny=2\nfn b\nx=11\ny=20\n"); - Ok(()) -} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index a19bfa2505..2224939afd 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -18,8 +18,6 @@ mod abort_tasks; #[cfg(not(target_os = "windows"))] mod apply_patch_cli; #[cfg(not(target_os = "windows"))] -mod apply_patch_freeform; -#[cfg(not(target_os = "windows"))] mod approvals; mod auth_refresh; mod cli_stream; diff --git a/codex-rs/protocol/src/items.rs b/codex-rs/protocol/src/items.rs index 27a9b94743..36ee0be07c 100644 --- a/codex-rs/protocol/src/items.rs +++ b/codex-rs/protocol/src/items.rs @@ -11,6 +11,8 @@ use serde::Serialize; use ts_rs::TS; #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)] +#[serde(tag = "type")] +#[ts(tag = "type")] pub enum TurnItem { UserMessage(UserMessageItem), AgentMessage(AgentMessageItem), @@ -25,6 +27,8 @@ pub struct UserMessageItem { } #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)] +#[serde(tag = "type")] +#[ts(tag = "type")] pub enum AgentMessageContent { Text { text: String }, } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 58732c3d4f..ae074b95fc 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -241,7 +241,7 @@ pub enum AskForApproval { /// Determines execution restrictions for model shell commands. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display, JsonSchema, TS)] #[strum(serialize_all = "kebab-case")] -#[serde(tag = "mode", rename_all = "kebab-case")] +#[serde(tag = "type", rename_all = "kebab-case")] pub enum SandboxPolicy { /// No restrictions whatsoever. Use with caution. #[serde(rename = "danger-full-access")] @@ -432,6 +432,7 @@ pub struct Event { /// NOTE: Make sure none of these values have optional types, as it will mess up the extension code-gen. #[derive(Debug, Clone, Deserialize, Serialize, Display, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] +#[ts(tag = "type")] #[strum(serialize_all = "snake_case")] pub enum EventMsg { /// Error while executing a submission @@ -617,6 +618,9 @@ pub struct ReasoningContentDeltaEvent { pub turn_id: String, pub item_id: String, pub delta: String, + // load with default value so it's backward compatible with the old format. + #[serde(default)] + pub summary_index: i64, } impl HasLegacyEvent for ReasoningContentDeltaEvent { @@ -633,6 +637,9 @@ pub struct ReasoningRawContentDeltaEvent { pub turn_id: String, pub item_id: String, pub delta: String, + // load with default value so it's backward compatible with the old format. + #[serde(default)] + pub content_index: i64, } impl HasLegacyEvent for ReasoningRawContentDeltaEvent { @@ -923,7 +930,13 @@ pub struct AgentReasoningRawContentDeltaEvent { } #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] -pub struct AgentReasoningSectionBreakEvent {} +pub struct AgentReasoningSectionBreakEvent { + // load with default value so it's backward compatible with the old format. + #[serde(default)] + pub item_id: String, + #[serde(default)] + pub summary_index: i64, +} #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct AgentReasoningDeltaEvent { @@ -1439,7 +1452,8 @@ pub enum ReviewDecision { } #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, JsonSchema, TS)] -#[serde(rename_all = "snake_case")] +#[serde(tag = "type", rename_all = "snake_case")] +#[ts(tag = "type")] pub enum FileChange { Add { content: String, diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index b245ea4eb4..fbc4c96b2e 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -100,6 +100,37 @@ fn upgrade_event_payload_for_tests(mut payload: serde_json::Value) -> serde_json serde_json::Value::String(aggregated), ); } + } else if ty == "patch_apply_begin" + && let Some(changes) = m.get_mut("changes").and_then(|v| v.as_object_mut()) + { + for change in changes.values_mut() { + if change.get("type").is_some() { + continue; + } + if let Some(change_obj) = change.as_object_mut() + && change_obj.len() == 1 + && let Some((legacy_kind, legacy_value)) = change_obj + .iter() + .next() + .map(|(k, v)| (k.clone(), v.clone())) + { + change_obj.clear(); + change_obj.insert( + "type".to_string(), + serde_json::Value::String(legacy_kind.clone()), + ); + match legacy_value { + serde_json::Value::Object(payload) => { + for (k, v) in payload { + change_obj.insert(k, v); + } + } + other => { + change_obj.insert("content".to_string(), other); + } + } + } + } } } payload