mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
remove flag for image preparation (#29429)
## What - make Fjord's centralized response-item image preparation unconditional for new and resumed history - have local user images and `view_image` outputs always defer decoding and resizing to that path - retain `resize_all_images` as an ignored, removed compatibility key for released clients - delete the flag-off producer paths and obsolete policy-specific tests ## Why Centralized preparation is now the intended image path. Keeping the runtime feature checks also kept two image-processing implementations alive and allowed client config to select the legacy behavior. This is a clean replacement for #28975, rebuilt from the latest `main`. ## How `prepare_response_items` now runs whenever items enter history and whenever persisted history is reconstructed. Producers emit deferred image data, so malformed images become the existing model-visible placeholder instead of failing the session at the producer. ## Test plan - `just fmt` - `just fix -p codex-core -p codex-features` - `just test -p codex-features` — 52 passed - focused affected `codex-core` set — 20 passed - `just test -p codex-core handle_accepts_explicit_high_detail` — 1 passed - full `just test -p codex-core` attempt — 2,723 passed; 88 unrelated environment failures from read-only `~/.codex` SQLite state and unavailable integration helper binaries
This commit is contained in:
1 parent
e98d43ac37
commit
e79d72d75d
18 files changed
+72
-221
No files matched your search
@@ -38,6 +38,8 @@ use tempfile::TempDir;
|
|||||||
use tokio::time::timeout;
|
use tokio::time::timeout;
|
||||||
use wiremock::MockServer;
|
use wiremock::MockServer;
|
||||||
|
|
||||||
|
const TINY_PNG_DATA_URL: &str = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
|
||||||
|
|
||||||
// macOS and Windows Bazel CI can spend tens of seconds starting app-server
|
// macOS and Windows Bazel CI can spend tens of seconds starting app-server
|
||||||
// subprocesses or processing test RPCs under load.
|
// subprocesses or processing test RPCs under load.
|
||||||
#[cfg(any(target_os = "macos", windows))]
|
#[cfg(any(target_os = "macos", windows))]
|
||||||
@@ -650,7 +652,7 @@ async fn dynamic_tool_call_round_trip_sends_content_items_to_model() -> Result<(
|
|||||||
text: "dynamic-ok".to_string(),
|
text: "dynamic-ok".to_string(),
|
||||||
},
|
},
|
||||||
DynamicToolCallOutputContentItem::InputImage {
|
DynamicToolCallOutputContentItem::InputImage {
|
||||||
image_url: "data:image/png;base64,AAA".to_string(),
|
image_url: TINY_PNG_DATA_URL.to_string(),
|
||||||
},
|
},
|
||||||
];
|
];
|
||||||
let content_items = response_content_items
|
let content_items = response_content_items
|
||||||
@@ -695,7 +697,7 @@ async fn dynamic_tool_call_round_trip_sends_content_items_to_model() -> Result<(
|
|||||||
text: "dynamic-ok".to_string(),
|
text: "dynamic-ok".to_string(),
|
||||||
},
|
},
|
||||||
DynamicToolCallOutputContentItem::InputImage {
|
DynamicToolCallOutputContentItem::InputImage {
|
||||||
image_url: "data:image/png;base64,AAA".to_string(),
|
image_url: TINY_PNG_DATA_URL.to_string(),
|
||||||
},
|
},
|
||||||
])
|
])
|
||||||
);
|
);
|
||||||
@@ -721,7 +723,7 @@ async fn dynamic_tool_call_round_trip_sends_content_items_to_model() -> Result<(
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
"type": "input_image",
|
"type": "input_image",
|
||||||
"image_url": "data:image/png;base64,AAA",
|
"image_url": TINY_PNG_DATA_URL,
|
||||||
"detail": "high"
|
"detail": "high"
|
||||||
}
|
}
|
||||||
])
|
])
|
||||||
|
|||||||
@@ -28,13 +28,13 @@ use wiremock::ResponseTemplate;
|
|||||||
use wiremock::matchers::method;
|
use wiremock::matchers::method;
|
||||||
use wiremock::matchers::path;
|
use wiremock::matchers::path;
|
||||||
|
|
||||||
const RESULT: &str = "cG5n";
|
const RESULT: &str = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
|
||||||
const TINY_PNG_BYTES: &[u8] = &[
|
const TINY_PNG_BYTES: &[u8] = &[
|
||||||
137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 1, 0, 0, 0, 1, 8, 6, 0,
|
137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 1, 0, 0, 0, 1, 8, 6, 0,
|
||||||
0, 0, 31, 21, 196, 137, 0, 0, 0, 11, 73, 68, 65, 84, 120, 156, 99, 96, 0, 2, 0, 0, 5, 0, 1,
|
0, 0, 31, 21, 196, 137, 0, 0, 0, 13, 73, 68, 65, 84, 120, 156, 99, 248, 207, 192, 240, 31, 0,
|
||||||
122, 94, 171, 63, 0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130,
|
5, 0, 1, 255, 137, 153, 61, 29, 0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130,
|
||||||
];
|
];
|
||||||
const TINY_PNG_DATA_URL: &str = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVR4nGNgAAIAAAUAAXpeqz8AAAAASUVORK5CYII=";
|
const TINY_PNG_DATA_URL: &str = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
|
||||||
|
|
||||||
#[derive(Clone, Copy)]
|
#[derive(Clone, Copy)]
|
||||||
enum ImagegenTestMode {
|
enum ImagegenTestMode {
|
||||||
@@ -116,7 +116,7 @@ async fn standalone_image_generation_returns_saved_path_hint_to_model() -> Resul
|
|||||||
assert_eq!(status, "completed");
|
assert_eq!(status, "completed");
|
||||||
assert_eq!(revised_prompt.as_deref(), Some("paint a blue whale"));
|
assert_eq!(revised_prompt.as_deref(), Some("paint a blue whale"));
|
||||||
assert_eq!(result, RESULT);
|
assert_eq!(result, RESULT);
|
||||||
assert_eq!(std::fs::read(&saved_path)?, b"png");
|
assert_eq!(std::fs::read(&saved_path)?, TINY_PNG_BYTES);
|
||||||
|
|
||||||
let requests = response_mock.requests();
|
let requests = response_mock.requests();
|
||||||
assert_eq!(requests.len(), 2);
|
assert_eq!(requests.len(), 2);
|
||||||
|
|||||||
@@ -81,7 +81,7 @@ pub(crate) async fn build_prompt_input_from_session(
|
|||||||
.await;
|
.await;
|
||||||
|
|
||||||
if !input.is_empty() {
|
if !input.is_empty() {
|
||||||
let response_item = sess.response_item_from_user_input(turn_context.as_ref(), input);
|
let response_item = sess.response_item_from_user_input(input);
|
||||||
sess.record_conversation_items(turn_context.as_ref(), std::slice::from_ref(&response_item))
|
sess.record_conversation_items(turn_context.as_ref(), std::slice::from_ref(&response_item))
|
||||||
.await;
|
.await;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1372,17 +1372,11 @@ impl Session {
|
|||||||
} = self
|
} = self
|
||||||
.reconstruct_history_from_rollout(turn_context, rollout_items)
|
.reconstruct_history_from_rollout(turn_context, rollout_items)
|
||||||
.await;
|
.await;
|
||||||
if turn_context
|
// Keep the recorded rollout unchanged. Prepare its reconstructed history before
|
||||||
.config
|
// installing it, so legacy images are processed once for this resume or fork and
|
||||||
.features
|
// will be processed again if the rollout is reconstructed in a future session.
|
||||||
.enabled(Feature::ResizeAllImages)
|
// This meets image resizing requirements without modifying persisted rollouts.
|
||||||
{
|
prepare_response_items(&mut history);
|
||||||
// Keep the recorded rollout unchanged. Prepare its reconstructed history before
|
|
||||||
// installing it, so legacy images are processed once for this resume or fork and
|
|
||||||
// will be processed again if the rollout is reconstructed in a future session.
|
|
||||||
// This meets image resizing requirements without modifying persisted rollouts.
|
|
||||||
prepare_response_items(&mut history);
|
|
||||||
}
|
|
||||||
{
|
{
|
||||||
let mut state = self.state.lock().await;
|
let mut state = self.state.lock().await;
|
||||||
state.replace_history(history, reference_context_item);
|
state.replace_history(history, reference_context_item);
|
||||||
@@ -2689,13 +2683,7 @@ impl Session {
|
|||||||
items: &'a [ResponseItem],
|
items: &'a [ResponseItem],
|
||||||
) -> Cow<'a, [ResponseItem]> {
|
) -> Cow<'a, [ResponseItem]> {
|
||||||
let mut items = Cow::Borrowed(items);
|
let mut items = Cow::Borrowed(items);
|
||||||
if turn_context
|
prepare_response_items(items.to_mut());
|
||||||
.config
|
|
||||||
.features
|
|
||||||
.enabled(Feature::ResizeAllImages)
|
|
||||||
{
|
|
||||||
prepare_response_items(items.to_mut());
|
|
||||||
}
|
|
||||||
if turn_context.config.features.enabled(Feature::ItemIds) {
|
if turn_context.config.features.enabled(Feature::ItemIds) {
|
||||||
Self::assign_missing_response_item_ids(items)
|
Self::assign_missing_response_item_ids(items)
|
||||||
} else {
|
} else {
|
||||||
@@ -2734,23 +2722,10 @@ impl Session {
|
|||||||
items
|
items
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn response_item_from_user_input(
|
pub(crate) fn response_item_from_user_input(&self, input: Vec<UserInput>) -> ResponseItem {
|
||||||
&self,
|
|
||||||
turn_context: &TurnContext,
|
|
||||||
input: Vec<UserInput>,
|
|
||||||
) -> ResponseItem {
|
|
||||||
let local_image_preparation = if turn_context
|
|
||||||
.config
|
|
||||||
.features
|
|
||||||
.enabled(Feature::ResizeAllImages)
|
|
||||||
{
|
|
||||||
LocalImagePreparation::Defer
|
|
||||||
} else {
|
|
||||||
LocalImagePreparation::Process
|
|
||||||
};
|
|
||||||
ResponseItem::from(ResponseInputItem::from_user_input(
|
ResponseItem::from(ResponseInputItem::from_user_input(
|
||||||
input,
|
input,
|
||||||
local_image_preparation,
|
LocalImagePreparation::Defer,
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -3666,7 +3641,7 @@ impl Session {
|
|||||||
// Persist the user message to history, but emit the turn item from `UserInput` so
|
// Persist the user message to history, but emit the turn item from `UserInput` so
|
||||||
// UI-only `text_elements` are preserved. `ResponseItem::Message` does not carry
|
// UI-only `text_elements` are preserved. `ResponseItem::Message` does not carry
|
||||||
// those spans, and `record_response_item_and_emit_turn_item` would drop them.
|
// those spans, and `record_response_item_and_emit_turn_item` would drop them.
|
||||||
let response_item = self.response_item_from_user_input(turn_context, input.to_vec());
|
let response_item = self.response_item_from_user_input(input.to_vec());
|
||||||
self.record_conversation_items(turn_context, std::slice::from_ref(&response_item))
|
self.record_conversation_items(turn_context, std::slice::from_ref(&response_item))
|
||||||
.await;
|
.await;
|
||||||
let mut user_message_item = UserMessageItem::new(input);
|
let mut user_message_item = UserMessageItem::new(input);
|
||||||
|
|||||||
@@ -1694,12 +1694,11 @@ async fn record_initial_history_reconstructs_resumed_transcript() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn resize_all_images_prepares_failures_before_history_insertion() {
|
async fn prepares_image_failures_before_history_insertion() {
|
||||||
let (session, turn_context, _rx) = make_session_and_context_with_auth_and_config_and_rx(
|
let (session, turn_context, _rx) = make_session_and_context_with_auth_and_config_and_rx(
|
||||||
CodexAuth::from_api_key("Test API Key"),
|
CodexAuth::from_api_key("Test API Key"),
|
||||||
Vec::new(),
|
Vec::new(),
|
||||||
|config| {
|
|config| {
|
||||||
let _ = config.features.enable(Feature::ResizeAllImages);
|
|
||||||
let _ = config.features.enable(Feature::ItemIds);
|
let _ = config.features.enable(Feature::ItemIds);
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
@@ -1764,15 +1763,8 @@ async fn resize_all_images_prepares_failures_before_history_insertion() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn resize_all_images_prepares_resumed_history_before_installing_it() {
|
async fn prepares_resumed_history_before_installing_it() {
|
||||||
let (session, _turn_context, _rx) = make_session_and_context_with_auth_and_config_and_rx(
|
let (session, _turn_context) = make_session_and_context().await;
|
||||||
CodexAuth::from_api_key("Test API Key"),
|
|
||||||
Vec::new(),
|
|
||||||
|config| {
|
|
||||||
let _ = config.features.enable(Feature::ResizeAllImages);
|
|
||||||
},
|
|
||||||
)
|
|
||||||
.await;
|
|
||||||
let resumed_item = ResponseItem::Message {
|
let resumed_item = ResponseItem::Message {
|
||||||
id: None,
|
id: None,
|
||||||
role: "user".to_string(),
|
role: "user".to_string(),
|
||||||
|
|||||||
@@ -1,4 +1,3 @@
|
|||||||
use codex_features::Feature;
|
|
||||||
use codex_protocol::items::ImageViewItem;
|
use codex_protocol::items::ImageViewItem;
|
||||||
use codex_protocol::items::TurnItem;
|
use codex_protocol::items::TurnItem;
|
||||||
use codex_protocol::models::DEFAULT_IMAGE_DETAIL;
|
use codex_protocol::models::DEFAULT_IMAGE_DETAIL;
|
||||||
@@ -8,9 +7,7 @@ use codex_protocol::models::FunctionCallOutputPayload;
|
|||||||
use codex_protocol::models::ImageDetail;
|
use codex_protocol::models::ImageDetail;
|
||||||
use codex_protocol::models::ResponseInputItem;
|
use codex_protocol::models::ResponseInputItem;
|
||||||
use codex_protocol::openai_models::InputModality;
|
use codex_protocol::openai_models::InputModality;
|
||||||
use codex_utils_image::PromptImageMode;
|
|
||||||
use codex_utils_image::data_url_from_bytes;
|
use codex_utils_image::data_url_from_bytes;
|
||||||
use codex_utils_image::load_for_prompt_bytes;
|
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
|
|
||||||
use crate::function_tool::FunctionCallError;
|
use crate::function_tool::FunctionCallError;
|
||||||
@@ -195,24 +192,8 @@ impl ViewImageHandler {
|
|||||||
DEFAULT_IMAGE_DETAIL
|
DEFAULT_IMAGE_DETAIL
|
||||||
};
|
};
|
||||||
|
|
||||||
let image_url = if turn.config.features.enabled(Feature::ResizeAllImages) {
|
// The history insertion path owns image decoding and resizing.
|
||||||
// The history insertion path owns image decoding and resizing when this is enabled.
|
let image_url = data_url_from_bytes("application/octet-stream", &file_bytes);
|
||||||
data_url_from_bytes("application/octet-stream", &file_bytes)
|
|
||||||
} else {
|
|
||||||
let image_mode = if use_original_detail {
|
|
||||||
PromptImageMode::Original
|
|
||||||
} else {
|
|
||||||
PromptImageMode::ResizeToFit
|
|
||||||
};
|
|
||||||
load_for_prompt_bytes(abs_path.as_path(), file_bytes, image_mode)
|
|
||||||
.map_err(|error| {
|
|
||||||
FunctionCallError::RespondToModel(format!(
|
|
||||||
"unable to process image at `{}`: {error}",
|
|
||||||
abs_path.display()
|
|
||||||
))
|
|
||||||
})?
|
|
||||||
.into_data_url()
|
|
||||||
};
|
|
||||||
|
|
||||||
let item = TurnItem::ImageView(ImageViewItem {
|
let item = TurnItem::ImageView(ImageViewItem {
|
||||||
id: call_id,
|
id: call_id,
|
||||||
@@ -420,9 +401,6 @@ mod tests {
|
|||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
|
|
||||||
let Err(FunctionCallError::RespondToModel(message)) = result else {
|
result.expect("explicit high detail should be accepted");
|
||||||
panic!("expected image processing error");
|
|
||||||
};
|
|
||||||
assert!(message.contains("unable to process image"), "{message}");
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -860,7 +860,7 @@ async fn resume_replays_legacy_js_repl_image_rollout_shapes() {
|
|||||||
async fn resume_replays_image_tool_outputs_with_detail() {
|
async fn resume_replays_image_tool_outputs_with_detail() {
|
||||||
skip_if_no_network!();
|
skip_if_no_network!();
|
||||||
|
|
||||||
let image_url = "data:image/webp;base64,UklGRiIAAABXRUJQVlA4IBYAAAAwAQCdASoBAAEAAUAmJaACdLoB+AADsAD+8ut//NgVzXPv9//S4P0uD9Lg/9KQAAA=";
|
let image_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
|
||||||
let function_call_id = "view-image-call";
|
let function_call_id = "view-image-call";
|
||||||
let custom_call_id = "js-repl-call";
|
let custom_call_id = "js-repl-call";
|
||||||
let thread_id = ThreadId::default();
|
let thread_id = ThreadId::default();
|
||||||
@@ -888,7 +888,7 @@ async fn resume_replays_image_tool_outputs_with_detail() {
|
|||||||
id: None,
|
id: None,
|
||||||
name: "view_image".to_string(),
|
name: "view_image".to_string(),
|
||||||
namespace: None,
|
namespace: None,
|
||||||
arguments: "{\"path\":\"/tmp/example.webp\"}".to_string(),
|
arguments: "{\"path\":\"/tmp/example.png\"}".to_string(),
|
||||||
call_id: function_call_id.to_string(),
|
call_id: function_call_id.to_string(),
|
||||||
metadata: None,
|
metadata: None,
|
||||||
}),
|
}),
|
||||||
|
|||||||
@@ -2681,7 +2681,7 @@ async fn code_mode_can_output_images_via_global_helper() -> Result<()> {
|
|||||||
&server,
|
&server,
|
||||||
"use exec to return images",
|
"use exec to return images",
|
||||||
r#"
|
r#"
|
||||||
image("data:image/png;base64,AAA");
|
image("data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==");
|
||||||
"#,
|
"#,
|
||||||
)
|
)
|
||||||
.await?;
|
.await?;
|
||||||
@@ -2706,7 +2706,7 @@ image("data:image/png;base64,AAA");
|
|||||||
items[1],
|
items[1],
|
||||||
serde_json::json!({
|
serde_json::json!({
|
||||||
"type": "input_image",
|
"type": "input_image",
|
||||||
"image_url": "data:image/png;base64,AAA",
|
"image_url": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==",
|
||||||
"detail": "high"
|
"detail": "high"
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
@@ -2715,17 +2715,14 @@ image("data:image/png;base64,AAA");
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn resize_all_images_replaces_malformed_code_mode_image() -> Result<()> {
|
async fn code_mode_replaces_malformed_image() -> Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
|
|
||||||
let server = responses::start_mock_server().await;
|
let server = responses::start_mock_server().await;
|
||||||
let (_test, second_mock) = run_code_mode_turn_with_config(
|
let (_test, second_mock) = run_code_mode_turn(
|
||||||
&server,
|
&server,
|
||||||
"use exec to return an image",
|
"use exec to return an image",
|
||||||
r#"image("data:image/png;base64,AAA");"#,
|
r#"image("data:image/png;base64,AAA");"#,
|
||||||
|config| {
|
|
||||||
let _ = config.features.enable(Feature::ResizeAllImages);
|
|
||||||
},
|
|
||||||
)
|
)
|
||||||
.await?;
|
.await?;
|
||||||
|
|
||||||
@@ -2746,7 +2743,7 @@ async fn resize_all_images_replaces_malformed_code_mode_image() -> Result<()> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn resize_all_images_resizes_explicit_original_code_mode_image() -> Result<()> {
|
async fn code_mode_resizes_explicit_original_image() -> Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
|
|
||||||
let original_dimensions = (6401, 100);
|
let original_dimensions = (6401, 100);
|
||||||
@@ -2772,12 +2769,7 @@ async fn resize_all_images_resizes_explicit_original_code_mode_image() -> Result
|
|||||||
"use exec to return a large original-detail image",
|
"use exec to return a large original-detail image",
|
||||||
&code,
|
&code,
|
||||||
"gpt-5.3-codex",
|
"gpt-5.3-codex",
|
||||||
|config| {
|
|_| {},
|
||||||
config
|
|
||||||
.features
|
|
||||||
.enable(Feature::ResizeAllImages)
|
|
||||||
.expect("resize_all_images should be enabled");
|
|
||||||
},
|
|
||||||
)
|
)
|
||||||
.await?;
|
.await?;
|
||||||
|
|
||||||
|
|||||||
@@ -4403,7 +4403,7 @@ async fn snapshot_request_shape_pre_turn_compaction_including_incoming_user_mess
|
|||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
.expect("override thread settings");
|
.expect("override thread settings");
|
||||||
let image_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR4nGNgYAAAAAMAASsJTYQAAAAASUVORK5CYII="
|
let image_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg=="
|
||||||
.to_string();
|
.to_string();
|
||||||
codex
|
codex
|
||||||
.submit(Op::UserInput {
|
.submit(Op::UserInput {
|
||||||
|
|||||||
@@ -42,9 +42,11 @@ use wiremock::matchers::path;
|
|||||||
|
|
||||||
const TINY_PNG_BYTES: &[u8] = &[
|
const TINY_PNG_BYTES: &[u8] = &[
|
||||||
137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 1, 0, 0, 0, 1, 8, 6, 0,
|
137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 1, 0, 0, 0, 1, 8, 6, 0,
|
||||||
0, 0, 31, 21, 196, 137, 0, 0, 0, 11, 73, 68, 65, 84, 120, 156, 99, 96, 0, 2, 0, 0, 5, 0, 1,
|
0, 0, 31, 21, 196, 137, 0, 0, 0, 13, 73, 68, 65, 84, 120, 156, 99, 248, 207, 192, 240, 31, 0,
|
||||||
122, 94, 171, 63, 0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130,
|
5, 0, 1, 255, 137, 153, 61, 29, 0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130,
|
||||||
];
|
];
|
||||||
|
const TINY_PNG_BASE64: &str = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
|
||||||
|
const TINY_PNG_DATA_URL: &str = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
|
||||||
|
|
||||||
fn image_generation_extensions(auth: &CodexAuth) -> Arc<ExtensionRegistry<Config>> {
|
fn image_generation_extensions(auth: &CodexAuth) -> Arc<ExtensionRegistry<Config>> {
|
||||||
let auth_manager = codex_core::test_support::auth_manager_from_auth(auth.clone());
|
let auth_manager = codex_core::test_support::auth_manager_from_auth(auth.clone());
|
||||||
@@ -148,7 +150,7 @@ async fn extension_tool_uses_granted_turn_permissions() -> Result<()> {
|
|||||||
.and(path("/v1/images/edits"))
|
.and(path("/v1/images/edits"))
|
||||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||||
"created": 1,
|
"created": 1,
|
||||||
"data": [{"b64_json": "cG5n"}],
|
"data": [{"b64_json": TINY_PNG_BASE64}],
|
||||||
})))
|
})))
|
||||||
.expect(1)
|
.expect(1)
|
||||||
.mount(&server)
|
.mount(&server)
|
||||||
@@ -299,7 +301,7 @@ async fn extension_tool_uses_granted_turn_permissions() -> Result<()> {
|
|||||||
let output = request.function_call_output(image_call_id);
|
let output = request.function_call_output(image_call_id);
|
||||||
let image = &output["output"][0];
|
let image = &output["output"][0];
|
||||||
assert_eq!(image["type"], "input_image");
|
assert_eq!(image["type"], "input_image");
|
||||||
assert_eq!(image["image_url"], "data:image/png;base64,cG5n");
|
assert_eq!(image["image_url"], TINY_PNG_DATA_URL);
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
@@ -200,7 +200,7 @@ async fn drag_drop_image_persists_rollout_request_shape() -> anyhow::Result<()>
|
|||||||
..
|
..
|
||||||
} = test_codex().build(&server).await?;
|
} = test_codex().build(&server).await?;
|
||||||
|
|
||||||
let image_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR4nGNgYAAAAAMAASsJTYQAAAAASUVORK5CYII=".to_string();
|
let image_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==".to_string();
|
||||||
|
|
||||||
let response = sse(vec![
|
let response = sse(vec![
|
||||||
ev_response_created("resp-1"),
|
ev_response_created("resp-1"),
|
||||||
|
|||||||
@@ -506,7 +506,7 @@ async fn model_change_from_image_to_text_strips_prior_image_content() -> Result<
|
|||||||
let _ = models_manager
|
let _ = models_manager
|
||||||
.list_models(RefreshStrategy::OnlineIfUncached)
|
.list_models(RefreshStrategy::OnlineIfUncached)
|
||||||
.await;
|
.await;
|
||||||
let image_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR4nGNgYAAAAAMAASsJTYQAAAAASUVORK5CYII="
|
let image_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg=="
|
||||||
.to_string();
|
.to_string();
|
||||||
|
|
||||||
test.codex
|
test.codex
|
||||||
|
|||||||
@@ -55,7 +55,7 @@ fn has_hosted_tool(tools: &[Value], tool_type: &str) -> bool {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn responses_lite_strips_data_image_detail_without_resize_all_images() -> Result<()> {
|
async fn responses_lite_strips_data_image_detail() -> Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
|
|
||||||
let server = responses::start_mock_server().await;
|
let server = responses::start_mock_server().await;
|
||||||
|
|||||||
@@ -24,7 +24,6 @@ use codex_core::config::Config;
|
|||||||
use codex_exec_server::CreateDirectoryOptions;
|
use codex_exec_server::CreateDirectoryOptions;
|
||||||
use codex_exec_server::Environment;
|
use codex_exec_server::Environment;
|
||||||
use codex_exec_server::HttpRequestParams;
|
use codex_exec_server::HttpRequestParams;
|
||||||
use codex_features::Feature;
|
|
||||||
use codex_login::CodexAuth;
|
use codex_login::CodexAuth;
|
||||||
use codex_mcp::MCP_SANDBOX_STATE_META_CAPABILITY;
|
use codex_mcp::MCP_SANDBOX_STATE_META_CAPABILITY;
|
||||||
use codex_models_manager::manager::RefreshStrategy;
|
use codex_models_manager::manager::RefreshStrategy;
|
||||||
@@ -1455,10 +1454,6 @@ async fn stdio_image_responses_resize_large_image() -> anyhow::Result<()> {
|
|||||||
let rmcp_test_server_bin = remote_aware_stdio_server_bin()?;
|
let rmcp_test_server_bin = remote_aware_stdio_server_bin()?;
|
||||||
let fixture = test_codex()
|
let fixture = test_codex()
|
||||||
.with_config(move |config| {
|
.with_config(move |config| {
|
||||||
config
|
|
||||||
.features
|
|
||||||
.enable(Feature::ResizeAllImages)
|
|
||||||
.expect("resize_all_images should be enabled");
|
|
||||||
insert_mcp_server(
|
insert_mcp_server(
|
||||||
config,
|
config,
|
||||||
server_name,
|
server_name,
|
||||||
|
|||||||
@@ -474,7 +474,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> {
|
|||||||
let rmcp_test_server_bin = stdio_server_bin()?;
|
let rmcp_test_server_bin = stdio_server_bin()?;
|
||||||
|
|
||||||
// 1x1 PNG data URL
|
// 1x1 PNG data URL
|
||||||
let openai_png = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMB/ee9bQAAAABJRU5ErkJggg==";
|
let openai_png = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
|
||||||
|
|
||||||
let mut builder = test_codex().with_config(move |config| {
|
let mut builder = test_codex().with_config(move |config| {
|
||||||
let mut servers = config.mcp_servers.get().clone();
|
let mut servers = config.mcp_servers.get().clone();
|
||||||
|
|||||||
@@ -7,7 +7,6 @@ use codex_exec_server::CreateDirectoryOptions;
|
|||||||
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
|
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
|
||||||
use codex_exec_server::REMOTE_ENVIRONMENT_ID;
|
use codex_exec_server::REMOTE_ENVIRONMENT_ID;
|
||||||
use codex_exec_server::RemoveOptions;
|
use codex_exec_server::RemoveOptions;
|
||||||
use codex_features::Feature;
|
|
||||||
use codex_login::CodexAuth;
|
use codex_login::CodexAuth;
|
||||||
use codex_protocol::config_types::ReasoningSummary;
|
use codex_protocol::config_types::ReasoningSummary;
|
||||||
use codex_protocol::models::PermissionProfile;
|
use codex_protocol::models::PermissionProfile;
|
||||||
@@ -181,15 +180,10 @@ async fn write_workspace_png(
|
|||||||
async fn assert_user_turn_local_image_resizes_to(
|
async fn assert_user_turn_local_image_resizes_to(
|
||||||
original_dimensions: (u32, u32),
|
original_dimensions: (u32, u32),
|
||||||
expected_dimensions: (u32, u32),
|
expected_dimensions: (u32, u32),
|
||||||
resize_policy: TestImageResizePolicy,
|
|
||||||
) -> anyhow::Result<()> {
|
) -> anyhow::Result<()> {
|
||||||
let server = start_mock_server().await;
|
let server = start_mock_server().await;
|
||||||
|
|
||||||
let mut builder = test_codex().with_config(move |config| {
|
let mut builder = test_codex();
|
||||||
if resize_policy == TestImageResizePolicy::AllImages {
|
|
||||||
let _ = config.features.enable(Feature::ResizeAllImages);
|
|
||||||
}
|
|
||||||
});
|
|
||||||
let test = builder.build_with_remote_env(&server).await?;
|
let test = builder.build_with_remote_env(&server).await?;
|
||||||
let TestCodex {
|
let TestCodex {
|
||||||
codex,
|
codex,
|
||||||
@@ -263,42 +257,25 @@ async fn assert_user_turn_local_image_resizes_to(
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Clone, Copy, Eq, PartialEq)]
|
|
||||||
enum TestImageResizePolicy {
|
|
||||||
Legacy,
|
|
||||||
AllImages,
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn user_turn_with_local_image_attaches_image() -> anyhow::Result<()> {
|
async fn user_turn_with_local_image_attaches_image() -> anyhow::Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
|
|
||||||
assert_user_turn_local_image_resizes_to((2304, 864), (2048, 768), TestImageResizePolicy::Legacy)
|
assert_user_turn_local_image_resizes_to((2304, 864), (2048, 768)).await
|
||||||
.await
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn user_turn_with_vertical_local_image_resizes_to_square_bounds() -> anyhow::Result<()> {
|
async fn user_turn_with_vertical_local_image_resizes_to_square_bounds() -> anyhow::Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
|
|
||||||
assert_user_turn_local_image_resizes_to(
|
assert_user_turn_local_image_resizes_to((1024, 4096), (512, 2048)).await
|
||||||
(1024, 4096),
|
|
||||||
(512, 2048),
|
|
||||||
TestImageResizePolicy::Legacy,
|
|
||||||
)
|
|
||||||
.await
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn resize_all_images_applies_patch_budget_to_local_user_image() -> anyhow::Result<()> {
|
async fn user_turn_local_image_applies_patch_budget() -> anyhow::Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
|
|
||||||
assert_user_turn_local_image_resizes_to(
|
assert_user_turn_local_image_resizes_to((2048, 2048), (1600, 1600)).await
|
||||||
(2048, 2048),
|
|
||||||
(1600, 1600),
|
|
||||||
TestImageResizePolicy::AllImages,
|
|
||||||
)
|
|
||||||
.await
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
@@ -1209,11 +1186,10 @@ async fn view_image_tool_errors_when_path_is_directory() -> anyhow::Result<()> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn view_image_tool_errors_for_non_image_files() -> anyhow::Result<()> {
|
async fn view_image_tool_turns_invalid_image_into_placeholder() -> anyhow::Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
|
|
||||||
let server = start_mock_server().await;
|
let server = start_mock_server().await;
|
||||||
|
|
||||||
let mut builder = test_codex();
|
let mut builder = test_codex();
|
||||||
let test = builder.build_with_remote_env(&server).await?;
|
let test = builder.build_with_remote_env(&server).await?;
|
||||||
let TestCodex {
|
let TestCodex {
|
||||||
@@ -1222,84 +1198,6 @@ async fn view_image_tool_errors_for_non_image_files() -> anyhow::Result<()> {
|
|||||||
..
|
..
|
||||||
} = &test;
|
} = &test;
|
||||||
|
|
||||||
let rel_path = "assets/example.json";
|
|
||||||
let abs_path =
|
|
||||||
write_workspace_file(&test, rel_path, br#"{ "message": "hello" }"#.to_vec()).await?;
|
|
||||||
|
|
||||||
let call_id = "view-image-non-image";
|
|
||||||
let arguments = serde_json::json!({ "path": rel_path }).to_string();
|
|
||||||
|
|
||||||
let first_response = sse(vec![
|
|
||||||
ev_response_created("resp-1"),
|
|
||||||
ev_function_call(call_id, "view_image", &arguments),
|
|
||||||
ev_completed("resp-1"),
|
|
||||||
]);
|
|
||||||
responses::mount_sse_once(&server, first_response).await;
|
|
||||||
|
|
||||||
let second_response = sse(vec![
|
|
||||||
ev_assistant_message("msg-1", "done"),
|
|
||||||
ev_completed("resp-2"),
|
|
||||||
]);
|
|
||||||
let mock = responses::mount_sse_once(&server, second_response).await;
|
|
||||||
|
|
||||||
let session_model = session_configured.model.clone();
|
|
||||||
|
|
||||||
codex
|
|
||||||
.submit(disabled_user_turn(
|
|
||||||
&test,
|
|
||||||
vec![UserInput::Text {
|
|
||||||
text: "please use the view_image tool to read the json file".into(),
|
|
||||||
text_elements: Vec::new(),
|
|
||||||
}],
|
|
||||||
session_model,
|
|
||||||
))
|
|
||||||
.await?;
|
|
||||||
|
|
||||||
wait_for_event_with_timeout(
|
|
||||||
codex,
|
|
||||||
|event| matches!(event, EventMsg::TurnComplete(_)),
|
|
||||||
VIEW_IMAGE_TURN_COMPLETE_TIMEOUT,
|
|
||||||
)
|
|
||||||
.await;
|
|
||||||
|
|
||||||
let request = mock.single_request();
|
|
||||||
assert!(
|
|
||||||
request.inputs_of_type("input_image").is_empty(),
|
|
||||||
"non-image file should not produce an input_image message"
|
|
||||||
);
|
|
||||||
let (error_text, success) = request
|
|
||||||
.function_call_output_content_and_success(call_id)
|
|
||||||
.expect("function_call_output should be present");
|
|
||||||
assert_eq!(success, None);
|
|
||||||
let error_text = error_text.expect("error text present");
|
|
||||||
|
|
||||||
let expected_error = format!(
|
|
||||||
"unable to process image at `{}`: unsupported image `application/json`",
|
|
||||||
abs_path.display()
|
|
||||||
);
|
|
||||||
assert!(
|
|
||||||
error_text.contains(&expected_error),
|
|
||||||
"error should describe unsupported file type: {error_text}"
|
|
||||||
);
|
|
||||||
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
||||||
async fn resize_all_images_turns_invalid_view_image_into_placeholder() -> anyhow::Result<()> {
|
|
||||||
skip_if_no_network!(Ok(()));
|
|
||||||
|
|
||||||
let server = start_mock_server().await;
|
|
||||||
let mut builder = test_codex().with_config(|config| {
|
|
||||||
let _ = config.features.enable(Feature::ResizeAllImages);
|
|
||||||
});
|
|
||||||
let test = builder.build_with_remote_env(&server).await?;
|
|
||||||
let TestCodex {
|
|
||||||
codex,
|
|
||||||
session_configured,
|
|
||||||
..
|
|
||||||
} = &test;
|
|
||||||
|
|
||||||
let rel_path = "assets/invalid-image.json";
|
let rel_path = "assets/invalid-image.json";
|
||||||
write_workspace_file(&test, rel_path, br#"{ "message": "hello" }"#.to_vec()).await?;
|
write_workspace_file(&test, rel_path, br#"{ "message": "hello" }"#.to_vec()).await?;
|
||||||
let call_id = "view-image-invalid-placeholder";
|
let call_id = "view-image-invalid-placeholder";
|
||||||
|
|||||||
@@ -193,7 +193,7 @@ pub enum Feature {
|
|||||||
ImageGeneration,
|
ImageGeneration,
|
||||||
/// Replace hosted image generation with the standalone image-generation extension.
|
/// Replace hosted image generation with the standalone image-generation extension.
|
||||||
ImageGenExt,
|
ImageGenExt,
|
||||||
/// Resize all inline data-URL images before recording them in history.
|
/// Removed compatibility flag for always-on centralized image preparation.
|
||||||
ResizeAllImages,
|
ResizeAllImages,
|
||||||
/// Generate Responses API item IDs for client-created history items.
|
/// Generate Responses API item IDs for client-created history items.
|
||||||
ItemIds,
|
ItemIds,
|
||||||
@@ -472,7 +472,7 @@ impl Features {
|
|||||||
"tool_search" | "apps_mcp_path_override" => {
|
"tool_search" | "apps_mcp_path_override" => {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
"image_detail_original" => {
|
"image_detail_original" | "resize_all_images" => {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
"plugin_hooks" => {
|
"plugin_hooks" => {
|
||||||
@@ -1160,8 +1160,8 @@ pub const FEATURES: &[FeatureSpec] = &[
|
|||||||
FeatureSpec {
|
FeatureSpec {
|
||||||
id: Feature::ResizeAllImages,
|
id: Feature::ResizeAllImages,
|
||||||
key: "resize_all_images",
|
key: "resize_all_images",
|
||||||
stage: Stage::UnderDevelopment,
|
stage: Stage::Removed,
|
||||||
default_enabled: false,
|
default_enabled: true,
|
||||||
},
|
},
|
||||||
FeatureSpec {
|
FeatureSpec {
|
||||||
id: Feature::ItemIds,
|
id: Feature::ItemIds,
|
||||||
|
|||||||
@@ -476,6 +476,23 @@ fn from_sources_ignores_removed_image_detail_original_feature_key() {
|
|||||||
assert_eq!(features, Features::with_defaults());
|
assert_eq!(features, Features::with_defaults());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn from_sources_ignores_removed_resize_all_images_feature_key() {
|
||||||
|
let features_toml =
|
||||||
|
FeaturesToml::from(BTreeMap::from([("resize_all_images".to_string(), false)]));
|
||||||
|
|
||||||
|
let features = Features::from_sources(
|
||||||
|
FeatureConfigSource {
|
||||||
|
features: Some(&features_toml),
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
FeatureConfigSource::default(),
|
||||||
|
FeatureOverrides::default(),
|
||||||
|
);
|
||||||
|
|
||||||
|
assert_eq!(features, Features::with_defaults());
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn from_sources_ignores_removed_undo_feature_key() {
|
fn from_sources_ignores_removed_undo_feature_key() {
|
||||||
let features_toml = FeaturesToml::from(BTreeMap::from([("undo".to_string(), true)]));
|
let features_toml = FeaturesToml::from(BTreeMap::from([("undo".to_string(), true)]));
|
||||||
|
|||||||
Reference in new issue
Block a user