From ccbae46e9667721881cb67b5aaf75c8cc983566a Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 18 Sep 2025 14:37:19 -0700 Subject: [PATCH 1/5] Move responses mocking helpers to a shared lib --- codex-rs/Cargo.lock | 1 + codex-rs/core/tests/common/Cargo.toml | 1 + codex-rs/core/tests/common/lib.rs | 2 + codex-rs/core/tests/common/responses.rs | 99 +++++++++++ codex-rs/core/tests/suite/compact.rs | 156 ++++-------------- .../core/tests/suite/compact_resume_fork.rs | 8 +- 6 files changed, 137 insertions(+), 130 deletions(-) create mode 100644 codex-rs/core/tests/common/responses.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 81fc8683cb..11a1641b8c 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1076,6 +1076,7 @@ dependencies = [ "serde_json", "tempfile", "tokio", + "wiremock", ] [[package]] diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index 9cfd20cdb4..2d43051919 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -11,3 +11,4 @@ codex-core = { path = "../.." } serde_json = "1" tempfile = "3" tokio = { version = "1", features = ["time"] } +wiremock = "0.6" diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index 244d093e7d..95af79b220 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -7,6 +7,8 @@ use codex_core::config::Config; use codex_core::config::ConfigOverrides; use codex_core::config::ConfigToml; +pub mod responses; + /// Returns a default `Config` whose on-disk state is confined to the provided /// temporary directory. Using a per-test directory keeps tests hermetic and /// avoids clobbering a developer’s real `~/.codex`. diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs new file mode 100644 index 0000000000..6a3717ed10 --- /dev/null +++ b/codex-rs/core/tests/common/responses.rs @@ -0,0 +1,99 @@ +use serde_json::Value; +use wiremock::matchers::{method, path}; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::BodyPrintLimit; +use wiremock::ResponseTemplate; + +/// Build an SSE stream body from a list of JSON events. +pub fn sse(events: Vec) -> String { + use std::fmt::Write as _; + let mut out = String::new(); + for ev in events { + let kind = ev.get("type").and_then(|v| v.as_str()).unwrap(); + writeln!(&mut out, "event: {kind}").unwrap(); + if !ev.as_object().map(|o| o.len() == 1).unwrap_or(false) { + write!(&mut out, "data: {ev}\n\n").unwrap(); + } else { + out.push('\n'); + } + } + out +} + +/// Convenience: SSE event for a completed response with a specific id. +pub fn ev_completed(id: &str) -> Value { + serde_json::json!({ + "type": "response.completed", + "response": { + "id": id, + "usage": {"input_tokens":0,"input_tokens_details":null,"output_tokens":0,"output_tokens_details":null,"total_tokens":0} + } + }) +} + +pub fn ev_completed_with_tokens(id: &str, total_tokens: u64) -> Value { + serde_json::json!({ + "type": "response.completed", + "response": { + "id": id, + "usage": { + "input_tokens": total_tokens, + "input_tokens_details": null, + "output_tokens": 0, + "output_tokens_details": null, + "total_tokens": total_tokens + } + } + }) +} + +/// Convenience: SSE event for a single assistant message output item. +pub fn ev_assistant_message(id: &str, text: &str) -> Value { + serde_json::json!({ + "type": "response.output_item.done", + "item": { + "type": "message", + "role": "assistant", + "id": id, + "content": [{"type": "output_text", "text": text}] + } + }) +} + +pub fn ev_function_call(call_id: &str, name: &str, arguments: &str) -> Value { + serde_json::json!({ + "type": "response.output_item.done", + "item": { + "type": "function_call", + "call_id": call_id, + "name": name, + "arguments": arguments + } + }) +} + +pub fn sse_response(body: String) -> ResponseTemplate { + ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw(body, "text/event-stream") +} + +pub async fn mount_sse_once(server: &MockServer, matcher: M, body: String) +where + M: wiremock::Match + Send + Sync + 'static, +{ + Mock::given(method("POST")) + .and(path("/v1/responses")) + .and(matcher) + .respond_with(sse_response(body)) + .mount(server) + .await; +} + +pub async fn start_mock_server() -> MockServer { + MockServer::builder() + .body_print_limit(BodyPrintLimit::Limited(80_000)) + .start() + .await +} \ No newline at end of file diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index 361315f724..297bbb783b 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -12,13 +12,10 @@ use codex_core::protocol::Op; use codex_core::protocol::RolloutItem; use codex_core::protocol::RolloutLine; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; -use core_test_support::load_default_config_for_test; +use core_test_support::{load_default_config_for_test, responses}; use core_test_support::wait_for_event; -use serde_json::Value; use tempfile::TempDir; -use wiremock::BodyPrintLimit; use wiremock::Mock; -use wiremock::MockServer; use wiremock::Request; use wiremock::Respond; use wiremock::ResponseTemplate; @@ -30,102 +27,9 @@ use std::sync::Arc; use std::sync::Mutex; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; - +use responses::{ev_assistant_message, ev_completed, sse, start_mock_server}; // --- Test helpers ----------------------------------------------------------- -/// Build an SSE stream body from a list of JSON events. -pub(super) fn sse(events: Vec) -> String { - use std::fmt::Write as _; - let mut out = String::new(); - for ev in events { - let kind = ev.get("type").and_then(|v| v.as_str()).unwrap(); - writeln!(&mut out, "event: {kind}").unwrap(); - if !ev.as_object().map(|o| o.len() == 1).unwrap_or(false) { - write!(&mut out, "data: {ev}\n\n").unwrap(); - } else { - out.push('\n'); - } - } - out -} - -/// Convenience: SSE event for a completed response with a specific id. -pub(super) fn ev_completed(id: &str) -> Value { - serde_json::json!({ - "type": "response.completed", - "response": { - "id": id, - "usage": {"input_tokens":0,"input_tokens_details":null,"output_tokens":0,"output_tokens_details":null,"total_tokens":0} - } - }) -} - -fn ev_completed_with_tokens(id: &str, total_tokens: u64) -> Value { - serde_json::json!({ - "type": "response.completed", - "response": { - "id": id, - "usage": { - "input_tokens": total_tokens, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": total_tokens - } - } - }) -} - -/// Convenience: SSE event for a single assistant message output item. -pub(super) fn ev_assistant_message(id: &str, text: &str) -> Value { - serde_json::json!({ - "type": "response.output_item.done", - "item": { - "type": "message", - "role": "assistant", - "id": id, - "content": [{"type": "output_text", "text": text}] - } - }) -} - -fn ev_function_call(call_id: &str, name: &str, arguments: &str) -> Value { - serde_json::json!({ - "type": "response.output_item.done", - "item": { - "type": "function_call", - "call_id": call_id, - "name": name, - "arguments": arguments - } - }) -} - -pub(super) fn sse_response(body: String) -> ResponseTemplate { - ResponseTemplate::new(200) - .insert_header("content-type", "text/event-stream") - .set_body_raw(body, "text/event-stream") -} - -pub(super) async fn mount_sse_once(server: &MockServer, matcher: M, body: String) -where - M: wiremock::Match + Send + Sync + 'static, -{ - Mock::given(method("POST")) - .and(path("/v1/responses")) - .and(matcher) - .respond_with(sse_response(body)) - .mount(server) - .await; -} - -async fn start_mock_server() -> MockServer { - MockServer::builder() - .body_print_limit(BodyPrintLimit::Limited(80_000)) - .start() - .await -} - pub(super) const FIRST_REPLY: &str = "FIRST_REPLY"; pub(super) const SUMMARY_TEXT: &str = "SUMMARY_ONLY_CONTEXT"; pub(super) const SUMMARIZE_TRIGGER: &str = "Start Summarization"; @@ -175,19 +79,19 @@ async fn summarize_context_three_requests_and_instructions() { body.contains("\"text\":\"hello world\"") && !body.contains(&format!("\"text\":\"{SUMMARIZE_TRIGGER}\"")) }; - mount_sse_once(&server, first_matcher, sse1).await; + responses::mount_sse_once(&server, first_matcher, sse1).await; let second_matcher = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); body.contains(&format!("\"text\":\"{SUMMARIZE_TRIGGER}\"")) }; - mount_sse_once(&server, second_matcher, sse2).await; + responses::mount_sse_once(&server, second_matcher, sse2).await; let third_matcher = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); body.contains(&format!("\"text\":\"{THIRD_USER_MSG}\"")) }; - mount_sse_once(&server, third_matcher, sse3).await; + responses::mount_sse_once(&server, third_matcher, sse3).await; // Build config pointing to the mock server and spawn Codex. let model_provider = ModelProviderInfo { @@ -381,17 +285,17 @@ async fn auto_compact_runs_after_token_limit_hit() { let sse1 = sse(vec![ ev_assistant_message("m1", FIRST_REPLY), - ev_completed_with_tokens("r1", 70_000), + responses::ev_completed_with_tokens("r1", 70_000), ]); let sse2 = sse(vec![ ev_assistant_message("m2", "SECOND_REPLY"), - ev_completed_with_tokens("r2", 330_000), + responses::ev_completed_with_tokens("r2", 330_000), ]); let sse3 = sse(vec![ ev_assistant_message("m3", AUTO_SUMMARY_TEXT), - ev_completed_with_tokens("r3", 200), + responses::ev_completed_with_tokens("r3", 200), ]); let first_matcher = |req: &wiremock::Request| { @@ -403,7 +307,7 @@ async fn auto_compact_runs_after_token_limit_hit() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(first_matcher) - .respond_with(sse_response(sse1)) + .respond_with(responses::sse_response(sse1)) .mount(&server) .await; @@ -416,7 +320,7 @@ async fn auto_compact_runs_after_token_limit_hit() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(second_matcher) - .respond_with(sse_response(sse2)) + .respond_with(responses::sse_response(sse2)) .mount(&server) .await; @@ -427,7 +331,7 @@ async fn auto_compact_runs_after_token_limit_hit() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(third_matcher) - .respond_with(sse_response(sse3)) + .respond_with(responses::sse_response(sse3)) .mount(&server) .await; @@ -522,17 +426,17 @@ async fn auto_compact_persists_rollout_entries() { let sse1 = sse(vec![ ev_assistant_message("m1", FIRST_REPLY), - ev_completed_with_tokens("r1", 70_000), + responses::ev_completed_with_tokens("r1", 70_000), ]); let sse2 = sse(vec![ ev_assistant_message("m2", "SECOND_REPLY"), - ev_completed_with_tokens("r2", 330_000), + responses::ev_completed_with_tokens("r2", 330_000), ]); let sse3 = sse(vec![ ev_assistant_message("m3", AUTO_SUMMARY_TEXT), - ev_completed_with_tokens("r3", 200), + responses::ev_completed_with_tokens("r3", 200), ]); let first_matcher = |req: &wiremock::Request| { @@ -544,7 +448,7 @@ async fn auto_compact_persists_rollout_entries() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(first_matcher) - .respond_with(sse_response(sse1)) + .respond_with(responses::sse_response(sse1)) .mount(&server) .await; @@ -557,7 +461,7 @@ async fn auto_compact_persists_rollout_entries() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(second_matcher) - .respond_with(sse_response(sse2)) + .respond_with(responses::sse_response(sse2)) .mount(&server) .await; @@ -568,7 +472,7 @@ async fn auto_compact_persists_rollout_entries() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(third_matcher) - .respond_with(sse_response(sse3)) + .respond_with(responses::sse_response(sse3)) .mount(&server) .await; @@ -655,17 +559,17 @@ async fn auto_compact_stops_after_failed_attempt() { let sse1 = sse(vec![ ev_assistant_message("m1", FIRST_REPLY), - ev_completed_with_tokens("r1", 500), + responses::ev_completed_with_tokens("r1", 500), ]); let sse2 = sse(vec![ ev_assistant_message("m2", SUMMARY_TEXT), - ev_completed_with_tokens("r2", 50), + responses::ev_completed_with_tokens("r2", 50), ]); let sse3 = sse(vec![ ev_assistant_message("m3", STILL_TOO_BIG_REPLY), - ev_completed_with_tokens("r3", 500), + responses::ev_completed_with_tokens("r3", 500), ]); let first_matcher = |req: &wiremock::Request| { @@ -676,7 +580,7 @@ async fn auto_compact_stops_after_failed_attempt() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(first_matcher) - .respond_with(sse_response(sse1.clone())) + .respond_with(responses::sse_response(sse1.clone())) .mount(&server) .await; @@ -687,7 +591,7 @@ async fn auto_compact_stops_after_failed_attempt() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(second_matcher) - .respond_with(sse_response(sse2.clone())) + .respond_with(responses::sse_response(sse2.clone())) .mount(&server) .await; @@ -699,7 +603,7 @@ async fn auto_compact_stops_after_failed_attempt() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(third_matcher) - .respond_with(sse_response(sse3.clone())) + .respond_with(responses::sse_response(sse3.clone())) .mount(&server) .await; @@ -769,27 +673,27 @@ async fn auto_compact_allows_multiple_attempts_when_interleaved_with_other_turn_ let sse1 = sse(vec![ ev_assistant_message("m1", FIRST_REPLY), - ev_completed_with_tokens("r1", 500), + responses::ev_completed_with_tokens("r1", 500), ]); let sse2 = sse(vec![ ev_assistant_message("m2", FIRST_AUTO_SUMMARY), - ev_completed_with_tokens("r2", 50), + responses::ev_completed_with_tokens("r2", 50), ]); let sse3 = sse(vec![ - ev_function_call(DUMMY_CALL_ID, DUMMY_FUNCTION_NAME, "{}"), - ev_completed_with_tokens("r3", 150), + responses::ev_function_call(DUMMY_CALL_ID, DUMMY_FUNCTION_NAME, "{}"), + responses::ev_completed_with_tokens("r3", 150), ]); let sse4 = sse(vec![ ev_assistant_message("m4", SECOND_LARGE_REPLY), - ev_completed_with_tokens("r4", 450), + responses::ev_completed_with_tokens("r4", 450), ]); let sse5 = sse(vec![ ev_assistant_message("m5", SECOND_AUTO_SUMMARY), - ev_completed_with_tokens("r5", 60), + responses::ev_completed_with_tokens("r5", 60), ]); let sse6 = sse(vec![ ev_assistant_message("m6", FINAL_REPLY), - ev_completed_with_tokens("r6", 120), + responses::ev_completed_with_tokens("r6", 120), ]); #[derive(Clone)] diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index 34b43ece91..f5c6a0909f 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -10,10 +10,10 @@ use super::compact::FIRST_REPLY; use super::compact::SUMMARIZE_TRIGGER; use super::compact::SUMMARY_TEXT; -use super::compact::ev_assistant_message; -use super::compact::ev_completed; -use super::compact::mount_sse_once; -use super::compact::sse; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::mount_sse_once; +use core_test_support::responses::sse; use codex_core::CodexAuth; use codex_core::CodexConversation; use codex_core::ConversationManager; From e8505ad331ccb113add3a7c805fd20110e781431 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 18 Sep 2025 14:41:24 -0700 Subject: [PATCH 2/5] test: reorder imports for clarity and fix grouping in compact and compact_resume_fork suite Reordered and grouped imports for consistency in test modules. No functional changes. --- codex-rs/core/tests/common/responses.rs | 7 ++++--- codex-rs/core/tests/suite/compact.rs | 8 ++++++-- codex-rs/core/tests/suite/compact_resume_fork.rs | 8 ++++---- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index 6a3717ed10..dc6f849ee7 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -1,9 +1,10 @@ use serde_json::Value; -use wiremock::matchers::{method, path}; +use wiremock::BodyPrintLimit; use wiremock::Mock; use wiremock::MockServer; -use wiremock::BodyPrintLimit; use wiremock::ResponseTemplate; +use wiremock::matchers::method; +use wiremock::matchers::path; /// Build an SSE stream body from a list of JSON events. pub fn sse(events: Vec) -> String { @@ -96,4 +97,4 @@ pub async fn start_mock_server() -> MockServer { .body_print_limit(BodyPrintLimit::Limited(80_000)) .start() .await -} \ No newline at end of file +} diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index 297bbb783b..286b6f3d56 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -12,7 +12,8 @@ use codex_core::protocol::Op; use codex_core::protocol::RolloutItem; use codex_core::protocol::RolloutLine; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; -use core_test_support::{load_default_config_for_test, responses}; +use core_test_support::load_default_config_for_test; +use core_test_support::responses; use core_test_support::wait_for_event; use tempfile::TempDir; use wiremock::Mock; @@ -23,11 +24,14 @@ use wiremock::matchers::method; use wiremock::matchers::path; use pretty_assertions::assert_eq; +use responses::ev_assistant_message; +use responses::ev_completed; +use responses::sse; +use responses::start_mock_server; use std::sync::Arc; use std::sync::Mutex; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; -use responses::{ev_assistant_message, ev_completed, sse, start_mock_server}; // --- Test helpers ----------------------------------------------------------- pub(super) const FIRST_REPLY: &str = "FIRST_REPLY"; diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index f5c6a0909f..23f9171d51 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -10,10 +10,6 @@ use super::compact::FIRST_REPLY; use super::compact::SUMMARIZE_TRIGGER; use super::compact::SUMMARY_TEXT; -use core_test_support::responses::ev_assistant_message; -use core_test_support::responses::ev_completed; -use core_test_support::responses::mount_sse_once; -use core_test_support::responses::sse; use codex_core::CodexAuth; use codex_core::CodexConversation; use codex_core::ConversationManager; @@ -27,6 +23,10 @@ use codex_core::protocol::InputItem; use codex_core::protocol::Op; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use core_test_support::load_default_config_for_test; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::mount_sse_once; +use core_test_support::responses::sse; use core_test_support::wait_for_event; use pretty_assertions::assert_eq; use serde_json::Value; From 2fc8688ed8a857813c5949ef0874b1a3a0345427 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 18 Sep 2025 14:49:36 -0700 Subject: [PATCH 3/5] tests: remove clippy unwrap_used lint expectation in compact.rs --- codex-rs/core/tests/suite/compact.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index 286b6f3d56..721a3719f6 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -1,5 +1,3 @@ -#![expect(clippy::unwrap_used)] - use codex_core::CodexAuth; use codex_core::ConversationManager; use codex_core::ModelProviderInfo; From 4997071d7414728c79cd3fa52497b2d647e4cbac Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 18 Sep 2025 16:59:07 -0700 Subject: [PATCH 4/5] Use helpers instead of fixtures --- codex-rs/core/tests/common/responses.rs | 33 +++++++++ .../tests/fixtures/sse_apply_patch_add.json | 25 ------- .../sse_apply_patch_freeform_add.json | 25 ------- .../sse_apply_patch_freeform_update.json | 25 ------- .../fixtures/sse_apply_patch_update.json | 25 ------- .../fixtures/sse_response_completed.json | 16 ----- codex-rs/exec/tests/suite/apply_patch.rs | 69 ++++++++++++++----- codex-rs/exec/tests/suite/common.rs | 6 +- 8 files changed, 85 insertions(+), 139 deletions(-) delete mode 100644 codex-rs/exec/tests/fixtures/sse_apply_patch_add.json delete mode 100644 codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_add.json delete mode 100644 codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_update.json delete mode 100644 codex-rs/exec/tests/fixtures/sse_apply_patch_update.json delete mode 100644 codex-rs/exec/tests/fixtures/sse_response_completed.json diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index dc6f849ee7..2f55f17a52 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -74,6 +74,39 @@ pub fn ev_function_call(call_id: &str, name: &str, arguments: &str) -> Value { }) } +/// 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). +pub fn ev_apply_patch_custom_tool_call(call_id: &str, patch: &str) -> Value { + serde_json::json!({ + "type": "response.output_item.done", + "item": { + "type": "custom_tool_call", + "name": "apply_patch", + "input": patch, + "call_id": call_id + } + }) +} + +/// Convenience: SSE event for an `apply_patch` function call. The Responses API +/// wraps the patch content in a JSON string under the `input` key; we recreate +/// the same structure so downstream code exercises the full parsing path. +pub fn ev_apply_patch_function_call(call_id: &str, patch: &str) -> Value { + let arguments = serde_json::json!({ "input": patch }); + let arguments = serde_json::to_string(&arguments).expect("serialize apply_patch arguments"); + + serde_json::json!({ + "type": "response.output_item.done", + "item": { + "type": "function_call", + "name": "apply_patch", + "arguments": arguments, + "call_id": call_id + } + }) +} + pub fn sse_response(body: String) -> ResponseTemplate { ResponseTemplate::new(200) .insert_header("content-type", "text/event-stream") diff --git a/codex-rs/exec/tests/fixtures/sse_apply_patch_add.json b/codex-rs/exec/tests/fixtures/sse_apply_patch_add.json deleted file mode 100644 index 8d2bf261af..0000000000 --- a/codex-rs/exec/tests/fixtures/sse_apply_patch_add.json +++ /dev/null @@ -1,25 +0,0 @@ -[ - { - "type": "response.output_item.done", - "item": { - "type": "custom_tool_call", - "name": "apply_patch", - "input": "*** Begin Patch\n*** Add File: test.md\n+Hello world\n*** End Patch", - "call_id": "__ID__" - } - }, - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -] diff --git a/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_add.json b/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_add.json deleted file mode 100644 index ce05e7d482..0000000000 --- a/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_add.json +++ /dev/null @@ -1,25 +0,0 @@ -[ - { - "type": "response.output_item.done", - "item": { - "type": "custom_tool_call", - "name": "apply_patch", - "input": "*** Begin Patch\n*** Add File: app.py\n+class BaseClass:\n+ def method():\n+ return False\n*** End Patch", - "call_id": "__ID__" - } - }, - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -] diff --git a/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_update.json b/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_update.json deleted file mode 100644 index 8329d9628c..0000000000 --- a/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_update.json +++ /dev/null @@ -1,25 +0,0 @@ -[ - { - "type": "response.output_item.done", - "item": { - "type": "custom_tool_call", - "name": "apply_patch", - "input": "*** Begin Patch\n*** Update File: app.py\n@@ def method():\n- return False\n+\n+ return True\n*** End Patch", - "call_id": "__ID__" - } - }, - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -] diff --git a/codex-rs/exec/tests/fixtures/sse_apply_patch_update.json b/codex-rs/exec/tests/fixtures/sse_apply_patch_update.json deleted file mode 100644 index 79689bece3..0000000000 --- a/codex-rs/exec/tests/fixtures/sse_apply_patch_update.json +++ /dev/null @@ -1,25 +0,0 @@ -[ - { - "type": "response.output_item.done", - "item": { - "type": "function_call", - "name": "apply_patch", - "arguments": "{\n \"input\": \"*** Begin Patch\\n*** Update File: test.md\\n@@\\n-Hello world\\n+Final text\\n*** End Patch\"\n}", - "call_id": "__ID__" - } - }, - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -] diff --git a/codex-rs/exec/tests/fixtures/sse_response_completed.json b/codex-rs/exec/tests/fixtures/sse_response_completed.json deleted file mode 100644 index 1774dc5e84..0000000000 --- a/codex-rs/exec/tests/fixtures/sse_response_completed.json +++ /dev/null @@ -1,16 +0,0 @@ -[ - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -] diff --git a/codex-rs/exec/tests/suite/apply_patch.rs b/codex-rs/exec/tests/suite/apply_patch.rs index 5537853b02..7ee262ad45 100644 --- a/codex-rs/exec/tests/suite/apply_patch.rs +++ b/codex-rs/exec/tests/suite/apply_patch.rs @@ -3,6 +3,10 @@ use anyhow::Context; use assert_cmd::prelude::*; use codex_core::CODEX_APPLY_PATCH_ARG1; +use core_test_support::responses::ev_apply_patch_custom_tool_call; +use core_test_support::responses::ev_apply_patch_function_call; +use core_test_support::responses::ev_completed; +use core_test_support::responses::sse; use std::fs; use std::process::Command; use tempfile::tempdir; @@ -55,15 +59,28 @@ async fn test_apply_patch_tool() -> anyhow::Result<()> { let tmp_cwd = tempdir().expect("failed to create temp dir"); let tmp_path = tmp_cwd.path().to_path_buf(); - run_e2e_exec_test( - tmp_cwd.path(), - vec![ - include_str!("../fixtures/sse_apply_patch_add.json").to_string(), - include_str!("../fixtures/sse_apply_patch_update.json").to_string(), - include_str!("../fixtures/sse_response_completed.json").to_string(), - ], - ) - .await; + let add_patch = r#"*** Begin Patch +*** Add File: test.md ++Hello world +*** End Patch"#; + let update_patch = r#"*** Begin Patch +*** Update File: test.md +@@ +-Hello world ++Final text +*** End Patch"#; + let response_streams = vec![ + sse(vec![ + ev_apply_patch_custom_tool_call("request_0", add_patch), + ev_completed("request_0"), + ]), + sse(vec![ + ev_apply_patch_function_call("request_1", update_patch), + ev_completed("request_1"), + ]), + sse(vec![ev_completed("request_2")]), + ]; + run_e2e_exec_test(tmp_cwd.path(), response_streams).await; let final_path = tmp_path.join("test.md"); let contents = std::fs::read_to_string(&final_path) @@ -86,15 +103,31 @@ async fn test_apply_patch_freeform_tool() -> anyhow::Result<()> { } let tmp_cwd = tempdir().expect("failed to create temp dir"); - run_e2e_exec_test( - tmp_cwd.path(), - vec![ - include_str!("../fixtures/sse_apply_patch_freeform_add.json").to_string(), - include_str!("../fixtures/sse_apply_patch_freeform_update.json").to_string(), - include_str!("../fixtures/sse_response_completed.json").to_string(), - ], - ) - .await; + let freeform_add_patch = r#"*** Begin Patch +*** Add File: app.py ++class BaseClass: ++ def method(): ++ return False +*** End Patch"#; + let freeform_update_patch = r#"*** Begin Patch +*** Update File: app.py +@@ def method(): +- return False ++ ++ return True +*** End Patch"#; + let response_streams = vec![ + sse(vec![ + ev_apply_patch_custom_tool_call("request_0", freeform_add_patch), + ev_completed("request_0"), + ]), + sse(vec![ + ev_apply_patch_custom_tool_call("request_1", freeform_update_patch), + ev_completed("request_1"), + ]), + sse(vec![ev_completed("request_2")]), + ]; + run_e2e_exec_test(tmp_cwd.path(), response_streams).await; // Verify final file contents let final_path = tmp_cwd.path().join("app.py"); diff --git a/codex-rs/exec/tests/suite/common.rs b/codex-rs/exec/tests/suite/common.rs index 4a3719aaba..19de9a3ef6 100644 --- a/codex-rs/exec/tests/suite/common.rs +++ b/codex-rs/exec/tests/suite/common.rs @@ -4,7 +4,6 @@ use anyhow::Context; use assert_cmd::prelude::*; -use core_test_support::load_sse_fixture_with_id_from_str; use std::path::Path; use std::process::Command; use std::sync::atomic::AtomicUsize; @@ -27,10 +26,7 @@ impl Respond for SeqResponder { match self.responses.get(call_num) { Some(body) => wiremock::ResponseTemplate::new(200) .insert_header("content-type", "text/event-stream") - .set_body_raw( - load_sse_fixture_with_id_from_str(body, &format!("request_{call_num}")), - "text/event-stream", - ), + .set_body_string(body.clone()), None => panic!("no response for {call_num}"), } } From 9f6e63176184f31dc414e2d6456e0fe4c3b21ce5 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 18 Sep 2025 17:22:13 -0700 Subject: [PATCH 5/5] tests(apply_patch): allow unused_imports in lint config --- codex-rs/exec/tests/suite/apply_patch.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/exec/tests/suite/apply_patch.rs b/codex-rs/exec/tests/suite/apply_patch.rs index 7ee262ad45..489f34f9ce 100644 --- a/codex-rs/exec/tests/suite/apply_patch.rs +++ b/codex-rs/exec/tests/suite/apply_patch.rs @@ -1,4 +1,4 @@ -#![allow(clippy::expect_used, clippy::unwrap_used)] +#![allow(clippy::expect_used, clippy::unwrap_used, unused_imports)] use anyhow::Context; use assert_cmd::prelude::*;