From 6b8175c7346d25a13479bc044819ca406ea1c3ae Mon Sep 17 00:00:00 2001 From: Won Park Date: Thu, 19 Mar 2026 15:16:26 -0700 Subject: [PATCH] changed save directory to codex_home (#15222) saving image gen default save directory to codex_home/imagegen/thread_id/ --- codex-rs/core/src/codex_tests.rs | 24 +++++- codex-rs/core/src/stream_events_utils.rs | 84 +++++++++++++++---- .../core/src/stream_events_utils_tests.rs | 61 +++++++++----- codex-rs/core/tests/suite/items.rs | 52 +++++++++++- codex-rs/core/tests/suite/model_switching.rs | 53 ++++++++++-- 5 files changed, 220 insertions(+), 54 deletions(-) diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index aca757ac9..d547b627a 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -3711,7 +3711,11 @@ async fn handle_output_item_done_records_image_save_history_message() { let session = Arc::new(session); let turn_context = Arc::new(turn_context); let call_id = "ig_history_records_message"; - let expected_saved_path = std::env::temp_dir().join(format!("{call_id}.png")); + let expected_saved_path = crate::stream_events_utils::image_generation_artifact_path( + turn_context.config.codex_home.as_path(), + &session.conversation_id.to_string(), + call_id, + ); let _ = std::fs::remove_file(&expected_saved_path); let item = ResponseItem::ImageGenerationCall { id: call_id.to_string(), @@ -3731,10 +3735,18 @@ async fn handle_output_item_done_records_image_save_history_message() { .expect("image generation item should succeed"); let history = session.clone_history().await; + let image_output_path = crate::stream_events_utils::image_generation_artifact_path( + turn_context.config.codex_home.as_path(), + &session.conversation_id.to_string(), + "", + ); + let image_output_dir = image_output_path + .parent() + .expect("generated image path should have a parent"); let save_message: ResponseItem = DeveloperInstructions::new(format!( "Generated images are saved to {} as {} by default.", - std::env::temp_dir().display(), - std::env::temp_dir().join(".png").display(), + image_output_dir.display(), + image_output_path.display(), )) .into(); assert_eq!(history.raw_items(), &[save_message, item]); @@ -3751,7 +3763,11 @@ async fn handle_output_item_done_skips_image_save_message_when_save_fails() { let session = Arc::new(session); let turn_context = Arc::new(turn_context); let call_id = "ig_history_no_message"; - let expected_saved_path = std::env::temp_dir().join(format!("{call_id}.png")); + let expected_saved_path = crate::stream_events_utils::image_generation_artifact_path( + turn_context.config.codex_home.as_path(), + &session.conversation_id.to_string(), + call_id, + ); let _ = std::fs::remove_file(&expected_saved_path); let item = ResponseItem::ImageGenerationCall { id: call_id.to_string(), diff --git a/codex-rs/core/src/stream_events_utils.rs b/codex-rs/core/src/stream_events_utils.rs index c5f784906..01b74f3a7 100644 --- a/codex-rs/core/src/stream_events_utils.rs +++ b/codex-rs/core/src/stream_events_utils.rs @@ -1,3 +1,4 @@ +use std::path::Path; use std::path::PathBuf; use std::pin::Pin; use std::sync::Arc; @@ -30,6 +31,36 @@ use futures::Future; use tracing::debug; use tracing::instrument; +const GENERATED_IMAGE_ARTIFACTS_DIR: &str = "generated_images"; + +pub(crate) fn image_generation_artifact_path( + codex_home: &Path, + session_id: &str, + call_id: &str, +) -> PathBuf { + let sanitize = |value: &str| { + let mut sanitized: String = value + .chars() + .map(|ch| { + if ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' { + ch + } else { + '_' + } + }) + .collect(); + if sanitized.is_empty() { + sanitized = "generated_image".to_string(); + } + sanitized + }; + + codex_home + .join(GENERATED_IMAGE_ARTIFACTS_DIR) + .join(sanitize(session_id)) + .join(format!("{}.png", sanitize(call_id))) +} + fn strip_hidden_assistant_markup(text: &str, plan_mode: bool) -> String { let (without_citations, _) = strip_citations(text); if plan_mode { @@ -71,26 +102,21 @@ pub(crate) fn raw_assistant_output_text_from_item(item: &ResponseItem) -> Option None } -async fn save_image_generation_result(call_id: &str, result: &str) -> Result { +async fn save_image_generation_result( + codex_home: &std::path::Path, + session_id: &str, + call_id: &str, + result: &str, +) -> Result { let bytes = BASE64_STANDARD .decode(result.trim().as_bytes()) .map_err(|err| { CodexErr::InvalidRequest(format!("invalid image generation payload: {err}")) })?; - let mut file_stem: String = call_id - .chars() - .map(|ch| { - if ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' { - ch - } else { - '_' - } - }) - .collect(); - if file_stem.is_empty() { - file_stem = "generated_image".to_string(); + let path = image_generation_artifact_path(codex_home, session_id, call_id); + if let Some(parent) = path.parent() { + tokio::fs::create_dir_all(parent).await?; } - let path = std::env::temp_dir().join(format!("{file_stem}.png")); tokio::fs::write(&path, bytes).await?; Ok(path) } @@ -321,14 +347,29 @@ pub(crate) async fn handle_non_tool_response_item( agent_message.memory_citation = memory_citation; } if let TurnItem::ImageGeneration(image_item) = &mut turn_item { - match save_image_generation_result(&image_item.id, &image_item.result).await { + let session_id = sess.conversation_id.to_string(); + match save_image_generation_result( + turn_context.config.codex_home.as_path(), + &session_id, + &image_item.id, + &image_item.result, + ) + .await + { Ok(path) => { image_item.saved_path = Some(path.to_string_lossy().into_owned()); - let image_output_dir = std::env::temp_dir(); + let image_output_path = image_generation_artifact_path( + turn_context.config.codex_home.as_path(), + &session_id, + "", + ); + let image_output_dir = image_output_path + .parent() + .unwrap_or(turn_context.config.codex_home.as_path()); let message: ResponseItem = DeveloperInstructions::new(format!( "Generated images are saved to {} as {} by default.", image_output_dir.display(), - image_output_dir.join(".png").display(), + image_output_path.display(), )) .into(); sess.record_conversation_items( @@ -338,7 +379,14 @@ pub(crate) async fn handle_non_tool_response_item( .await; } Err(err) => { - let output_dir = std::env::temp_dir(); + let output_path = image_generation_artifact_path( + turn_context.config.codex_home.as_path(), + &session_id, + &image_item.id, + ); + let output_dir = output_path + .parent() + .unwrap_or(turn_context.config.codex_home.as_path()); tracing::warn!( call_id = %image_item.id, output_dir = %output_dir.display(), diff --git a/codex-rs/core/src/stream_events_utils_tests.rs b/codex-rs/core/src/stream_events_utils_tests.rs index 389f01ec7..3757577b5 100644 --- a/codex-rs/core/src/stream_events_utils_tests.rs +++ b/codex-rs/core/src/stream_events_utils_tests.rs @@ -1,4 +1,5 @@ use super::handle_non_tool_response_item; +use super::image_generation_artifact_path; use super::last_assistant_message_from_item; use super::save_image_generation_result; use crate::codex::make_session_and_context; @@ -80,13 +81,16 @@ fn last_assistant_message_from_item_returns_none_for_plan_only_hidden_message() } #[tokio::test] -async fn save_image_generation_result_saves_base64_to_png_in_temp_dir() { - let expected_path = std::env::temp_dir().join("ig_save_base64.png"); +async fn save_image_generation_result_saves_base64_to_png_in_codex_home() { + let codex_home = tempfile::tempdir().expect("create codex home"); + let expected_path = + image_generation_artifact_path(codex_home.path(), "session-1", "ig_save_base64"); let _ = std::fs::remove_file(&expected_path); - let saved_path = save_image_generation_result("ig_save_base64", "Zm9v") - .await - .expect("image should be saved"); + let saved_path = + save_image_generation_result(codex_home.path(), "session-1", "ig_save_base64", "Zm9v") + .await + .expect("image should be saved"); assert_eq!(saved_path, expected_path); assert_eq!(std::fs::read(&saved_path).expect("saved file"), b"foo"); @@ -96,8 +100,9 @@ async fn save_image_generation_result_saves_base64_to_png_in_temp_dir() { #[tokio::test] async fn save_image_generation_result_rejects_data_url_payload() { let result = "data:image/jpeg;base64,Zm9v"; + let codex_home = tempfile::tempdir().expect("create codex home"); - let err = save_image_generation_result("ig_456", result) + let err = save_image_generation_result(codex_home.path(), "session-1", "ig_456", result) .await .expect_err("data url payload should error"); assert!(matches!(err, CodexErr::InvalidRequest(_))); @@ -105,12 +110,21 @@ async fn save_image_generation_result_rejects_data_url_payload() { #[tokio::test] async fn save_image_generation_result_overwrites_existing_file() { - let existing_path = std::env::temp_dir().join("ig_overwrite.png"); + let codex_home = tempfile::tempdir().expect("create codex home"); + let existing_path = + image_generation_artifact_path(codex_home.path(), "session-1", "ig_overwrite"); + std::fs::create_dir_all( + existing_path + .parent() + .expect("generated image path should have a parent"), + ) + .expect("create image output dir"); std::fs::write(&existing_path, b"existing").expect("seed existing image"); - let saved_path = save_image_generation_result("ig_overwrite", "Zm9v") - .await - .expect("image should be saved"); + let saved_path = + save_image_generation_result(codex_home.path(), "session-1", "ig_overwrite", "Zm9v") + .await + .expect("image should be saved"); assert_eq!(saved_path, existing_path); assert_eq!(std::fs::read(&saved_path).expect("saved file"), b"foo"); @@ -118,13 +132,15 @@ async fn save_image_generation_result_overwrites_existing_file() { } #[tokio::test] -async fn save_image_generation_result_sanitizes_call_id_for_temp_dir_output_path() { - let expected_path = std::env::temp_dir().join("___ig___.png"); +async fn save_image_generation_result_sanitizes_call_id_for_codex_home_output_path() { + let codex_home = tempfile::tempdir().expect("create codex home"); + let expected_path = image_generation_artifact_path(codex_home.path(), "session-1", "../ig/.."); let _ = std::fs::remove_file(&expected_path); - let saved_path = save_image_generation_result("../ig/..", "Zm9v") - .await - .expect("image should be saved"); + let saved_path = + save_image_generation_result(codex_home.path(), "session-1", "../ig/..", "Zm9v") + .await + .expect("image should be saved"); assert_eq!(saved_path, expected_path); assert_eq!(std::fs::read(&saved_path).expect("saved file"), b"foo"); @@ -133,7 +149,8 @@ async fn save_image_generation_result_sanitizes_call_id_for_temp_dir_output_path #[tokio::test] async fn save_image_generation_result_rejects_non_standard_base64() { - let err = save_image_generation_result("ig_urlsafe", "_-8") + let codex_home = tempfile::tempdir().expect("create codex home"); + let err = save_image_generation_result(codex_home.path(), "session-1", "ig_urlsafe", "_-8") .await .expect_err("non-standard base64 should error"); assert!(matches!(err, CodexErr::InvalidRequest(_))); @@ -141,8 +158,14 @@ async fn save_image_generation_result_rejects_non_standard_base64() { #[tokio::test] async fn save_image_generation_result_rejects_non_base64_data_urls() { - let err = save_image_generation_result("ig_svg", "data:image/svg+xml,") - .await - .expect_err("non-base64 data url should error"); + let codex_home = tempfile::tempdir().expect("create codex home"); + let err = save_image_generation_result( + codex_home.path(), + "session-1", + "ig_svg", + "data:image/svg+xml,", + ) + .await + .expect_err("non-base64 data url should error"); assert!(matches!(err, CodexErr::InvalidRequest(_))); } diff --git a/codex-rs/core/tests/suite/items.rs b/codex-rs/core/tests/suite/items.rs index 113a94601..d86054555 100644 --- a/codex-rs/core/tests/suite/items.rs +++ b/codex-rs/core/tests/suite/items.rs @@ -35,6 +35,32 @@ use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use core_test_support::wait_for_event_match; use pretty_assertions::assert_eq; +use std::path::Path; +use std::path::PathBuf; + +fn image_generation_artifact_path(codex_home: &Path, session_id: &str, call_id: &str) -> PathBuf { + fn sanitize(value: &str) -> String { + let mut sanitized: String = value + .chars() + .map(|ch| { + if ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' { + ch + } else { + '_' + } + }) + .collect(); + if sanitized.is_empty() { + sanitized = "generated_image".to_string(); + } + sanitized + } + + codex_home + .join("generated_images") + .join(sanitize(session_id)) + .join(format!("{}.png", sanitize(call_id))) +} #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn user_message_item_is_emitted() -> anyhow::Result<()> { @@ -269,9 +295,18 @@ async fn image_generation_call_event_is_emitted() -> anyhow::Result<()> { let server = start_mock_server().await; - let TestCodex { codex, .. } = test_codex().build(&server).await?; + let TestCodex { + codex, + config, + session_configured, + .. + } = test_codex().build(&server).await?; let call_id = "ig_image_saved_to_temp_dir_default"; - let expected_saved_path = std::env::temp_dir().join(format!("{call_id}.png")); + let expected_saved_path = image_generation_artifact_path( + config.codex_home.as_path(), + &session_configured.session_id.to_string(), + call_id, + ); let _ = std::fs::remove_file(&expected_saved_path); let first_response = sse(vec![ @@ -323,8 +358,17 @@ async fn image_generation_call_event_is_emitted_when_image_save_fails() -> anyho let server = start_mock_server().await; - let TestCodex { codex, .. } = test_codex().build(&server).await?; - let expected_saved_path = std::env::temp_dir().join("ig_invalid.png"); + let TestCodex { + codex, + config, + session_configured, + .. + } = test_codex().build(&server).await?; + let expected_saved_path = image_generation_artifact_path( + config.codex_home.as_path(), + &session_configured.session_id.to_string(), + "ig_invalid", + ); let _ = std::fs::remove_file(&expected_saved_path); let first_response = sse(vec![ diff --git a/codex-rs/core/tests/suite/model_switching.rs b/codex-rs/core/tests/suite/model_switching.rs index 748fce023..c6e21c9ee 100644 --- a/codex-rs/core/tests/suite/model_switching.rs +++ b/codex-rs/core/tests/suite/model_switching.rs @@ -32,8 +32,34 @@ use core_test_support::skip_if_no_network; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use pretty_assertions::assert_eq; +use std::path::Path; +use std::path::PathBuf; use wiremock::MockServer; +fn image_generation_artifact_path(codex_home: &Path, session_id: &str, call_id: &str) -> PathBuf { + fn sanitize(value: &str) -> String { + let mut sanitized: String = value + .chars() + .map(|ch| { + if ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' { + ch + } else { + '_' + } + }) + .collect(); + if sanitized.is_empty() { + sanitized = "generated_image".to_string(); + } + sanitized + } + + codex_home + .join("generated_images") + .join(sanitize(session_id)) + .join(format!("{}.png", sanitize(call_id))) +} + fn test_model_info( slug: &str, display_name: &str, @@ -444,9 +470,6 @@ async fn model_change_from_image_to_text_strips_prior_image_content() -> Result< async fn generated_image_is_replayed_for_image_capable_models() -> Result<()> { skip_if_no_network!(Ok(())); - let saved_path = std::env::temp_dir().join("ig_123.png"); - let _ = std::fs::remove_file(&saved_path); - let server = MockServer::start().await; let image_model_slug = "test-image-model"; let image_model = test_model_info( @@ -482,6 +505,12 @@ async fn generated_image_is_replayed_for_image_capable_models() -> Result<()> { config.model = Some(image_model_slug.to_string()); }); let test = builder.build(&server).await?; + let saved_path = image_generation_artifact_path( + test.codex_home_path(), + &test.session_configured.session_id.to_string(), + "ig_123", + ); + let _ = std::fs::remove_file(&saved_path); let models_manager = test.thread_manager.get_models_manager(); let _ = models_manager .list_models(RefreshStrategy::OnlineIfUncached) @@ -564,9 +593,6 @@ async fn model_change_from_generated_image_to_text_preserves_prior_generated_ima -> Result<()> { skip_if_no_network!(Ok(())); - let saved_path = std::env::temp_dir().join("ig_123.png"); - let _ = std::fs::remove_file(&saved_path); - let server = MockServer::start().await; let image_model_slug = "test-image-model"; let text_model_slug = "test-text-only-model"; @@ -609,6 +635,12 @@ async fn model_change_from_generated_image_to_text_preserves_prior_generated_ima config.model = Some(image_model_slug.to_string()); }); let test = builder.build(&server).await?; + let saved_path = image_generation_artifact_path( + test.codex_home_path(), + &test.session_configured.session_id.to_string(), + "ig_123", + ); + let _ = std::fs::remove_file(&saved_path); let models_manager = test.thread_manager.get_models_manager(); let _ = models_manager .list_models(RefreshStrategy::OnlineIfUncached) @@ -700,9 +732,6 @@ async fn model_change_from_generated_image_to_text_preserves_prior_generated_ima async fn thread_rollback_after_generated_image_drops_entire_image_turn_history() -> Result<()> { skip_if_no_network!(Ok(())); - let saved_path = std::env::temp_dir().join("ig_rollback.png"); - let _ = std::fs::remove_file(&saved_path); - let server = MockServer::start().await; let image_model_slug = "test-image-model"; let image_model = test_model_info( @@ -738,6 +767,12 @@ async fn thread_rollback_after_generated_image_drops_entire_image_turn_history() config.model = Some(image_model_slug.to_string()); }); let test = builder.build(&server).await?; + let saved_path = image_generation_artifact_path( + test.codex_home_path(), + &test.session_configured.session_id.to_string(), + "ig_rollback", + ); + let _ = std::fs::remove_file(&saved_path); let models_manager = test.thread_manager.get_models_manager(); let _ = models_manager .list_models(RefreshStrategy::OnlineIfUncached)