From dc3deaa3e7fe584872ddf2855da13111c8a5242f Mon Sep 17 00:00:00 2001 From: jif-oai Date: Wed, 14 Jan 2026 09:07:45 +0000 Subject: [PATCH] feat: return an error if the image sent by the user is a bad image (#9146) ## Before When we detect an `InvalidImageRequest`, we replace the image by a placeholder and keep going ## Now In such `InvalidImageRequest`, we check if the image is due to a user message or a tool call output. For tool call output we still replace it with a placeholder to avoid breaking the agentic loop bu tif this is because of a user message, we send an error to the user --- codex-rs/core/src/codex.rs | 13 ++++- codex-rs/core/src/context_manager/history.rs | 33 ++++++------ .../core/src/context_manager/history_tests.rs | 53 +++++++++++++++++++ 3 files changed, 81 insertions(+), 18 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 8d9ef9c44..ea032fd54 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2629,9 +2629,18 @@ pub(crate) async fn run_turn( Err(CodexErr::InvalidImageRequest()) => { let mut state = sess.state.lock().await; error_or_panic( - "Invalid image detected, replacing it in the last turn to prevent poisoning", + "Invalid image detected; sanitizing tool output to prevent poisoning", ); - state.history.replace_last_turn_images("Invalid image"); + if state.history.replace_last_turn_images("Invalid image") { + continue; + } + let event = EventMsg::Error(ErrorEvent { + message: "Invalid image in your last message. Please remove it and try again." + .to_string(), + codex_error_info: Some(CodexErrorInfo::BadRequest), + }); + sess.send_event(&turn_context, event).await; + break; } Err(e) => { info!("Turn error: {e:#}"); diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index 3e0428c86..4ee2e252c 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -124,34 +124,35 @@ impl ContextManager { self.items = items; } - pub(crate) fn replace_last_turn_images(&mut self, placeholder: &str) { - let Some(last_item) = self.items.last_mut() else { - return; + /// Replace image content in the last turn if it originated from a tool output. + /// Returns true when a tool image was replaced, false otherwise. + pub(crate) fn replace_last_turn_images(&mut self, placeholder: &str) -> bool { + let Some(index) = self.items.iter().rposition(|item| { + matches!(item, ResponseItem::FunctionCallOutput { .. }) + || matches!(item, ResponseItem::Message { role, .. } if role == "user") + }) else { + return false; }; - match last_item { - ResponseItem::Message { role, content, .. } if role == "user" => { - for item in content.iter_mut() { - if matches!(item, ContentItem::InputImage { .. }) { - *item = ContentItem::InputText { - text: placeholder.to_string(), - }; - } - } - } + match &mut self.items[index] { ResponseItem::FunctionCallOutput { output, .. } => { let Some(content_items) = output.content_items.as_mut() else { - return; + return false; }; + let mut replaced = false; + let placeholder = placeholder.to_string(); for item in content_items.iter_mut() { if matches!(item, FunctionCallOutputContentItem::InputImage { .. }) { *item = FunctionCallOutputContentItem::InputText { - text: placeholder.to_string(), + text: placeholder.clone(), }; + replaced = true; } } + replaced } - _ => {} + ResponseItem::Message { role, .. } if role == "user" => false, + _ => false, } } diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index eca2a1889..b3d061bab 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -3,6 +3,7 @@ use crate::truncate; use crate::truncate::TruncationPolicy; use codex_git::GhostCommit; use codex_protocol::models::ContentItem; +use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::LocalShellAction; use codex_protocol::models::LocalShellExecAction; @@ -209,6 +210,58 @@ fn remove_first_item_removes_matching_call_for_output() { assert_eq!(h.raw_items(), vec![]); } +#[test] +fn replace_last_turn_images_replaces_tool_output_images() { + let items = vec![ + user_input_text_msg("hi"), + ResponseItem::FunctionCallOutput { + call_id: "call-1".to_string(), + output: FunctionCallOutputPayload { + content: "ok".to_string(), + content_items: Some(vec![FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,AAA".to_string(), + }]), + success: Some(true), + }, + }, + ]; + let mut history = create_history_with_items(items); + + assert!(history.replace_last_turn_images("Invalid image")); + + assert_eq!( + history.raw_items(), + vec![ + user_input_text_msg("hi"), + ResponseItem::FunctionCallOutput { + call_id: "call-1".to_string(), + output: FunctionCallOutputPayload { + content: "ok".to_string(), + content_items: Some(vec![FunctionCallOutputContentItem::InputText { + text: "Invalid image".to_string(), + }]), + success: Some(true), + }, + }, + ] + ); +} + +#[test] +fn replace_last_turn_images_does_not_touch_user_images() { + let items = vec![ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputImage { + image_url: "data:image/png;base64,AAA".to_string(), + }], + }]; + let mut history = create_history_with_items(items.clone()); + + assert!(!history.replace_last_turn_images("Invalid image")); + assert_eq!(history.raw_items(), items); +} + #[test] fn remove_first_item_handles_local_shell_pair() { let items = vec![