Skip to content

Commit 2893476

Browse files
authored
unifying all image saves to /tmp to bug-proof (#14149)
image-gen feature will have the model saving to /tmp by default + at all times
1 parent 2895d35 commit 2893476

File tree

8 files changed

+214
-82
lines changed

8 files changed

+214
-82
lines changed

codex-rs/core/src/codex.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6811,8 +6811,7 @@ async fn handle_assistant_item_done_in_plan_mode(
68116811
{
68126812
maybe_complete_plan_item_from_message(sess, turn_context, state, item).await;
68136813

6814-
if let Some(turn_item) =
6815-
handle_non_tool_response_item(item, true, Some(&turn_context.cwd)).await
6814+
if let Some(turn_item) = handle_non_tool_response_item(sess, turn_context, item, true).await
68166815
{
68176816
emit_turn_item_in_plan_mode(
68186817
sess,
@@ -6993,8 +6992,13 @@ async fn try_run_sampling_request(
69936992
needs_follow_up |= output_result.needs_follow_up;
69946993
}
69956994
ResponseEvent::OutputItemAdded(item) => {
6996-
if let Some(turn_item) =
6997-
handle_non_tool_response_item(&item, plan_mode, Some(&turn_context.cwd)).await
6995+
if let Some(turn_item) = handle_non_tool_response_item(
6996+
sess.as_ref(),
6997+
turn_context.as_ref(),
6998+
&item,
6999+
plan_mode,
7000+
)
7001+
.await
69987002
{
69997003
let mut turn_item = turn_item;
70007004
let mut seeded_parsed: Option<ParsedAssistantTextDelta> = None;

codex-rs/core/src/codex_tests.rs

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,26 @@ fn developer_input_texts(items: &[ResponseItem]) -> Vec<&str> {
154154
.collect()
155155
}
156156

157+
fn default_image_save_developer_message_text() -> String {
158+
let image_output_dir = crate::stream_events_utils::default_image_generation_output_dir();
159+
format!(
160+
"Generated images are saved to {} as {} by default.",
161+
image_output_dir.display(),
162+
image_output_dir.join("<image_id>.png").display(),
163+
)
164+
}
165+
166+
fn test_tool_runtime(session: Arc<Session>, turn_context: Arc<TurnContext>) -> ToolCallRuntime {
167+
let router = Arc::new(ToolRouter::from_config(
168+
&turn_context.tools_config,
169+
None,
170+
None,
171+
turn_context.dynamic_tools.as_slice(),
172+
));
173+
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
174+
ToolCallRuntime::new(router, session, turn_context, tracker)
175+
}
176+
157177
fn make_connector(id: &str, name: &str) -> AppInfo {
158178
AppInfo {
159179
id: id.to_string(),
@@ -3123,6 +3143,114 @@ async fn build_initial_context_uses_previous_realtime_state() {
31233143
);
31243144
}
31253145

3146+
#[tokio::test]
3147+
async fn build_initial_context_omits_default_image_save_location_with_image_history() {
3148+
let (session, turn_context) = make_session_and_context().await;
3149+
session
3150+
.replace_history(
3151+
vec![ResponseItem::ImageGenerationCall {
3152+
id: "ig-test".to_string(),
3153+
status: "completed".to_string(),
3154+
revised_prompt: Some("a tiny blue square".to_string()),
3155+
result: "Zm9v".to_string(),
3156+
}],
3157+
None,
3158+
)
3159+
.await;
3160+
3161+
let initial_context = session.build_initial_context(&turn_context).await;
3162+
let developer_texts = developer_input_texts(&initial_context);
3163+
assert!(
3164+
!developer_texts
3165+
.iter()
3166+
.any(|text| text.contains("Generated images are saved to")),
3167+
"expected initial context to omit image save instructions even with image history, got {developer_texts:?}"
3168+
);
3169+
}
3170+
3171+
#[tokio::test]
3172+
async fn build_initial_context_omits_default_image_save_location_without_image_history() {
3173+
let (session, turn_context) = make_session_and_context().await;
3174+
3175+
let initial_context = session.build_initial_context(&turn_context).await;
3176+
let developer_texts = developer_input_texts(&initial_context);
3177+
3178+
assert!(
3179+
!developer_texts
3180+
.iter()
3181+
.any(|text| text.contains("Generated images are saved to")),
3182+
"expected initial context to omit image save instructions without image history, got {developer_texts:?}"
3183+
);
3184+
}
3185+
3186+
#[tokio::test]
3187+
async fn handle_output_item_done_records_image_save_message_after_successful_save() {
3188+
let (session, turn_context) = make_session_and_context().await;
3189+
let session = Arc::new(session);
3190+
let turn_context = Arc::new(turn_context);
3191+
let call_id = "ig_history_records_message";
3192+
let expected_saved_path = crate::stream_events_utils::default_image_generation_output_dir()
3193+
.join(format!("{call_id}.png"));
3194+
let _ = std::fs::remove_file(&expected_saved_path);
3195+
let item = ResponseItem::ImageGenerationCall {
3196+
id: call_id.to_string(),
3197+
status: "completed".to_string(),
3198+
revised_prompt: Some("a tiny blue square".to_string()),
3199+
result: "Zm9v".to_string(),
3200+
};
3201+
3202+
let mut ctx = HandleOutputCtx {
3203+
sess: Arc::clone(&session),
3204+
turn_context: Arc::clone(&turn_context),
3205+
tool_runtime: test_tool_runtime(Arc::clone(&session), Arc::clone(&turn_context)),
3206+
cancellation_token: CancellationToken::new(),
3207+
};
3208+
handle_output_item_done(&mut ctx, item.clone(), None)
3209+
.await
3210+
.expect("image generation item should succeed");
3211+
3212+
let history = session.clone_history().await;
3213+
let expected_message: ResponseItem =
3214+
DeveloperInstructions::new(default_image_save_developer_message_text()).into();
3215+
assert_eq!(history.raw_items(), &[expected_message, item]);
3216+
assert_eq!(
3217+
std::fs::read(&expected_saved_path).expect("saved file"),
3218+
b"foo"
3219+
);
3220+
let _ = std::fs::remove_file(&expected_saved_path);
3221+
}
3222+
3223+
#[tokio::test]
3224+
async fn handle_output_item_done_skips_image_save_message_when_save_fails() {
3225+
let (session, turn_context) = make_session_and_context().await;
3226+
let session = Arc::new(session);
3227+
let turn_context = Arc::new(turn_context);
3228+
let call_id = "ig_history_no_message";
3229+
let expected_saved_path = crate::stream_events_utils::default_image_generation_output_dir()
3230+
.join(format!("{call_id}.png"));
3231+
let _ = std::fs::remove_file(&expected_saved_path);
3232+
let item = ResponseItem::ImageGenerationCall {
3233+
id: call_id.to_string(),
3234+
status: "completed".to_string(),
3235+
revised_prompt: Some("broken payload".to_string()),
3236+
result: "_-8".to_string(),
3237+
};
3238+
3239+
let mut ctx = HandleOutputCtx {
3240+
sess: Arc::clone(&session),
3241+
turn_context: Arc::clone(&turn_context),
3242+
tool_runtime: test_tool_runtime(Arc::clone(&session), Arc::clone(&turn_context)),
3243+
cancellation_token: CancellationToken::new(),
3244+
};
3245+
handle_output_item_done(&mut ctx, item.clone(), None)
3246+
.await
3247+
.expect("image generation item should still complete");
3248+
3249+
let history = session.clone_history().await;
3250+
assert_eq!(history.raw_items(), &[item]);
3251+
assert!(!expected_saved_path.exists());
3252+
}
3253+
31263254
#[tokio::test]
31273255
async fn build_initial_context_uses_previous_turn_settings_for_realtime_end() {
31283256
let (session, turn_context) = make_session_and_context().await;

codex-rs/core/src/context_manager/history_tests.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -434,9 +434,6 @@ fn for_prompt_rewrites_image_generation_calls_when_images_are_supported() {
434434
ContentItem::InputImage {
435435
image_url: "data:image/png;base64,Zm9v".to_string(),
436436
},
437-
ContentItem::InputText {
438-
text: "Saved to: CWD".to_string(),
439-
},
440437
],
441438
end_turn: None,
442439
phase: None,
@@ -503,9 +500,6 @@ fn for_prompt_rewrites_image_generation_calls_when_images_are_unsupported() {
503500
text: "image content omitted because you do not support image input"
504501
.to_string(),
505502
},
506-
ContentItem::InputText {
507-
text: "Saved to: CWD".to_string(),
508-
},
509503
],
510504
end_turn: None,
511505
phase: None,

codex-rs/core/src/context_manager/normalize.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,9 +242,6 @@ pub(crate) fn rewrite_image_generation_calls_for_stateless_input(items: &mut Vec
242242
text: format!("Prompt: {revised_prompt}"),
243243
},
244244
ContentItem::InputImage { image_url },
245-
ContentItem::InputText {
246-
text: "Saved to: CWD".to_string(),
247-
},
248245
],
249246
end_turn: None,
250247
phase: None,

0 commit comments

Comments
 (0)