diff --git a/codex-rs/code-mode-protocol/src/response.rs b/codex-rs/code-mode-protocol/src/response.rs index 9b45032d94..6ff96bd331 100644 --- a/codex-rs/code-mode-protocol/src/response.rs +++ b/codex-rs/code-mode-protocol/src/response.rs @@ -1,17 +1,8 @@ +pub use codex_protocol::models::DEFAULT_IMAGE_DETAIL; +pub use codex_protocol::models::ImageDetail; use serde::Deserialize; use serde::Serialize; -#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] -#[serde(rename_all = "lowercase")] -pub enum ImageDetail { - Auto, - Low, - High, - Original, -} - -pub const DEFAULT_IMAGE_DETAIL: ImageDetail = ImageDetail::High; - #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] #[serde(tag = "type", rename_all = "snake_case")] pub enum FunctionCallOutputContentItem { diff --git a/codex-rs/code-mode-runtime/BUILD.bazel b/codex-rs/code-mode-runtime/BUILD.bazel index 22de889d3a..16382b60ca 100644 --- a/codex-rs/code-mode-runtime/BUILD.bazel +++ b/codex-rs/code-mode-runtime/BUILD.bazel @@ -2,6 +2,9 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "code-mode-runtime", - compile_data = ["src/runtime/settled.js"], + compile_data = [ + "src/runtime/content.js", + "src/runtime/settled.js", + ], crate_name = "codex_code_mode_runtime", ) diff --git a/codex-rs/code-mode-runtime/src/runtime/callbacks.rs b/codex-rs/code-mode-runtime/src/runtime/callbacks.rs index 0bccbc5a11..bc97826cff 100644 --- a/codex-rs/code-mode-runtime/src/runtime/callbacks.rs +++ b/codex-rs/code-mode-runtime/src/runtime/callbacks.rs @@ -6,6 +6,7 @@ use super::RuntimeState; use super::timers; use super::value::json_to_v8; use super::value::normalize_output_audio; +use super::value::normalize_output_content; use super::value::normalize_output_image; use super::value::serialize_output_text; use super::value::throw_type_error; @@ -93,16 +94,21 @@ pub(super) fn text_callback( return; } }; - if let Some(state) = scope.get_slot::() - && !state.exit_requested - { - let _ = state - .event_tx - .blocking_send(RuntimeEvent::ContentItem(OutputItem::InputText { text })); - } + emit_output_item(scope, OutputItem::InputText { text }); retval.set(v8::undefined(scope).into()); } +pub(super) fn content_block_callback( + scope: &mut v8::PinScope<'_, '_>, + args: v8::FunctionCallbackArguments, + _retval: v8::ReturnValue, +) { + match normalize_output_content(scope, args.get(0)) { + Ok(item) => emit_output_item(scope, item), + Err(message) => throw_type_error(scope, &message), + } +} + pub(super) fn audio_callback( scope: &mut v8::PinScope<'_, '_>, args: v8::FunctionCallbackArguments, @@ -117,13 +123,7 @@ pub(super) fn audio_callback( Ok(audio_item) => audio_item, Err(()) => return, }; - if let Some(state) = scope.get_slot::() - && !state.exit_requested - { - let _ = state - .event_tx - .blocking_send(RuntimeEvent::ContentItem(audio_item)); - } + emit_output_item(scope, audio_item); retval.set(v8::undefined(scope).into()); } @@ -137,31 +137,23 @@ pub(super) fn image_callback( } else { args.get(0) }; - let detail_override = if args.length() < 2 { - None - } else { - let detail = args.get(1); - if detail.is_string() { - Some(detail.to_rust_string_lossy(scope)) - } else if detail.is_null() || detail.is_undefined() { - None - } else { - throw_type_error(scope, "image detail must be a string when provided"); - return; - } - }; + let detail_override = (args.length() > 1).then(|| args.get(1)); let image_item = match normalize_output_image(scope, value, detail_override) { Ok(image_item) => image_item, Err(()) => return, }; + emit_output_item(scope, image_item); + retval.set(v8::undefined(scope).into()); +} + +fn emit_output_item(scope: &v8::PinScope<'_, '_>, item: OutputItem) { if let Some(state) = scope.get_slot::() && !state.exit_requested { let _ = state .event_tx - .blocking_send(RuntimeEvent::ContentItem(image_item)); + .blocking_send(RuntimeEvent::ContentItem(item)); } - retval.set(v8::undefined(scope).into()); } pub(super) fn generated_image_callback( diff --git a/codex-rs/code-mode-runtime/src/runtime/callbacks_tests.rs b/codex-rs/code-mode-runtime/src/runtime/callbacks_tests.rs index 772ba2261e..06e410d631 100644 --- a/codex-rs/code-mode-runtime/src/runtime/callbacks_tests.rs +++ b/codex-rs/code-mode-runtime/src/runtime/callbacks_tests.rs @@ -41,6 +41,7 @@ async fn host_callbacks_admit_no_new_effects_after_exit_is_requested() { r#" markExit(); text("still working"); +content({type: "text", text: "late"}); image("data:image/png;base64,AAA"); audio("data:audio/mpeg;base64,YXVkaW8="); generatedImage({image_url: "data:image/png;base64,AAA", output_hint: "late"}); @@ -53,6 +54,14 @@ clearTimeout(earlyTimer); setTimeout(() => {}, 60000); "text conversion", r#"text({toJSON() { return markExit("late"); }});"#, ), + ( + "content getter", + r#"content({type: "text", get text() { return markExit("late"); }});"#, + ), + ( + "content iterator", + r#"content((function* () { yield markExit({type: "text", text: "late"}); })());"#, + ), ( "notify conversion", r#"notify({toJSON() { return markExit("late"); }});"#, diff --git a/codex-rs/code-mode-runtime/src/runtime/content.js b/codex-rs/code-mode-runtime/src/runtime/content.js new file mode 100644 index 0000000000..0aaf3ae65f --- /dev/null +++ b/codex-rs/code-mode-runtime/src/runtime/content.js @@ -0,0 +1,20 @@ +(() => { + const emitBlock = globalThis.content; + const iteratorKey = Symbol.iterator; + const apply = Reflect.apply; + + globalThis.content = function content(value) { + const iterate = + typeof value === "string" ? undefined : value?.[iteratorKey]; + if (typeof iterate !== "function") { + return emitBlock(value); + } + + // Reuse the method from a potentially stateful getter, with its original + // receiver. The for-of loop closes the iterator if the host rejects a block. + const blocks = { [iteratorKey]: () => apply(iterate, value, []) }; + for (const block of blocks) { + emitBlock(block); + } + }; +})(); diff --git a/codex-rs/code-mode-runtime/src/runtime/globals.rs b/codex-rs/code-mode-runtime/src/runtime/globals.rs index 1c39bb4317..4e844ef88e 100644 --- a/codex-rs/code-mode-runtime/src/runtime/globals.rs +++ b/codex-rs/code-mode-runtime/src/runtime/globals.rs @@ -1,5 +1,6 @@ use super::callbacks::audio_callback; use super::callbacks::clear_timeout_callback; +use super::callbacks::content_block_callback; use super::callbacks::exit_callback; use super::callbacks::generated_image_callback; use super::callbacks::image_callback; @@ -27,6 +28,7 @@ pub(super) fn install_globals( let clear_timeout = helper_function(scope, "clearTimeout", clear_timeout_callback)?; let set_timeout = helper_function(scope, "setTimeout", set_timeout_callback)?; let text = helper_function(scope, "text", text_callback)?; + let content_block = helper_function(scope, "content", content_block_callback)?; let image = helper_function(scope, "image", image_callback)?; let audio = helper_function(scope, "audio", audio_callback)?; let generated_image = helper_function(scope, "generatedImage", generated_image_callback)?; @@ -41,6 +43,7 @@ pub(super) fn install_globals( set_global(scope, global, "clearTimeout", clear_timeout.into())?; set_global(scope, global, "setTimeout", set_timeout.into())?; set_global(scope, global, "text", text.into())?; + set_global(scope, global, "content", content_block.into())?; set_global(scope, global, "image", image.into())?; set_global(scope, global, "audio", audio.into())?; set_global(scope, global, "generatedImage", generated_image.into())?; @@ -50,11 +53,12 @@ pub(super) fn install_globals( set_global(scope, global, "yield_control", yield_control.into())?; set_global(scope, global, "exit", exit.into())?; - let source = v8::String::new(scope, include_str!("settled.js")) - .ok_or_else(|| "failed to allocate settlement helpers".to_string())?; + let helpers = concat!(include_str!("settled.js"), "\n", include_str!("content.js")); + let source = v8::String::new(scope, helpers) + .ok_or_else(|| "failed to allocate JavaScript helpers".to_string())?; v8::Script::compile(scope, source, /*origin*/ None) .and_then(|script| script.run(scope)) - .ok_or_else(|| "failed to initialize settlement helpers".to_string())?; + .ok_or_else(|| "failed to initialize JavaScript helpers".to_string())?; Ok(()) } diff --git a/codex-rs/code-mode-runtime/src/runtime/value.rs b/codex-rs/code-mode-runtime/src/runtime/value.rs index 8451aefc1c..6c93ad9cef 100644 --- a/codex-rs/code-mode-runtime/src/runtime/value.rs +++ b/codex-rs/code-mode-runtime/src/runtime/value.rs @@ -3,10 +3,13 @@ use serde_json::Value as JsonValue; use crate::session_runtime::OutputItem; use codex_code_mode_protocol::DEFAULT_IMAGE_DETAIL; use codex_code_mode_protocol::ImageDetail; +use codex_protocol::models::FunctionCallOutputContentItem; +use codex_protocol::models::ImageReference; use super::RuntimeState; use super::audio::wav_duration_seconds; +const CONTENT_HELPER_EXPECTS_MESSAGE: &str = "content expects one MCP ContentBlock"; const IMAGE_HELPER_EXPECTS_MESSAGE: &str = "image expects a non-empty image URL string, an object with image_url and optional detail, or a raw MCP image block"; const AUDIO_HELPER_EXPECTS_MESSAGE: &str = "audio expects a non-empty audio URL string, an object with audio_url, or a raw MCP audio block"; const REMOTE_IMAGE_URL_ERROR: &str = "Tool call failed: remote image URLs are not supported in tool outputs. Pass a base64 data URI instead"; @@ -14,7 +17,6 @@ const INVALID_IMAGE_URL_ERROR: &str = "Tool call failed: invalid image output. Pass a base64 data URI instead"; const INVALID_AUDIO_URL_ERROR: &str = "Tool call failed: invalid audio output. Pass a base64 data URI instead"; -const CODEX_IMAGE_DETAIL_META_KEY: &str = "codex/imageDetail"; pub(super) fn serialize_output_text( scope: &mut v8::PinScope<'_, '_>, @@ -30,35 +32,65 @@ pub(super) fn serialize_output_text( return Ok(value.to_rust_string_lossy(scope)); } + if let Some(text) = stringify_json_value(scope, value)? { + return Ok(text); + } let tc = std::pin::pin!(v8::TryCatch::new(scope)); - let mut tc = tc.init(); - if let Some(stringified) = v8::json::stringify(&tc, value) { - return Ok(stringified.to_rust_string_lossy(&tc)); + Ok(value.to_rust_string_lossy(&tc.init())) +} + +pub(super) fn normalize_output_content( + scope: &mut v8::PinScope<'_, '_>, + value: v8::Local<'_, v8::Value>, +) -> Result { + let block = v8_value_to_json(scope, value)? + .filter(JsonValue::is_object) + .ok_or_else(|| CONTENT_HELPER_EXPECTS_MESSAGE.to_string())?; + mcp_output_item(&block) +} + +fn mcp_output_item(block: &JsonValue) -> Result { + let item = FunctionCallOutputContentItem::from_mcp_content_block(block) + .map_err(|_| CONTENT_HELPER_EXPECTS_MESSAGE.to_string())?; + match item { + FunctionCallOutputContentItem::InputText { text } => Ok(OutputItem::InputText { text }), + FunctionCallOutputContentItem::InputImage { + image: ImageReference::Inline { image_url }, + detail, + } => Ok(OutputItem::InputImage { image_url, detail }), + FunctionCallOutputContentItem::InputAudio { audio_url } => Ok(output_audio_item(audio_url)), + FunctionCallOutputContentItem::EncryptedContent { .. } => { + Err("content cannot emit encrypted MCP text".to_string()) + } + FunctionCallOutputContentItem::InputImage { + image: ImageReference::File { .. }, + .. + } => Err("content expects inline MCP image data".to_string()), } - if tc.has_caught() { - return Err(tc - .exception() - .map(|exception| value_to_error_text(&mut tc, exception)) - .unwrap_or_else(|| "unknown code mode exception".to_string())); - } - Ok(value.to_rust_string_lossy(&tc)) } pub(super) fn normalize_output_image( scope: &mut v8::PinScope<'_, '_>, value: v8::Local<'_, v8::Value>, - detail_override: Option, + detail_override: Option>, ) -> Result { let result = (|| -> Result { - let (image_url, detail) = if value.is_string() { + let mut explicit_detail = parse_image_detail_value(scope, detail_override)?; + let (image_url, mcp_detail) = if value.is_string() { (value.to_rust_string_lossy(scope), None) } else if value.is_object() && !value.is_array() { let object = v8::Local::::try_from(value) .map_err(|_| IMAGE_HELPER_EXPECTS_MESSAGE.to_string())?; - if let Some(image) = parse_non_mcp_output_image(scope, object)? { - image + if let Some((image_url, detail)) = parse_non_mcp_output_image(scope, object)? { + explicit_detail = explicit_detail.or(detail); + (image_url, None) } else { - parse_mcp_output_image(scope, value)? + let OutputItem::InputImage { image_url, detail } = + parse_mcp_media_block(scope, value, "image", IMAGE_HELPER_EXPECTS_MESSAGE)? + else { + return Err(IMAGE_HELPER_EXPECTS_MESSAGE.to_string()); + }; + (image_url, detail) } } else { return Err(IMAGE_HELPER_EXPECTS_MESSAGE.to_string()); @@ -77,26 +109,18 @@ pub(super) fn normalize_output_image( return Err(INVALID_IMAGE_URL_ERROR.to_string()); } - let detail = detail_override.or(detail); - let detail = match detail { - Some(detail) => { - let normalized = detail.to_ascii_lowercase(); - Some(match normalized.as_str() { - "auto" => ImageDetail::Auto, - "low" => ImageDetail::Low, - "high" => ImageDetail::High, - "original" => ImageDetail::Original, - _ => { - return Err( - "image detail must be one of: auto, low, high, original".to_string() - ); - } - }) - } - None => Some(DEFAULT_IMAGE_DETAIL), + let detail = if let Some(mut detail) = explicit_detail { + detail.make_ascii_lowercase(); + serde_json::from_value::(JsonValue::String(detail)) + .map_err(|_| "image detail must be one of: auto, low, high, original".to_string())? + } else { + mcp_detail.unwrap_or(DEFAULT_IMAGE_DETAIL) }; - Ok(OutputItem::InputImage { image_url, detail }) + Ok(OutputItem::InputImage { + image_url, + detail: Some(detail), + }) })(); match result { @@ -129,56 +153,47 @@ fn parse_non_mcp_output_image( Ok(Some((image_url.to_rust_string_lossy(scope), detail))) } -fn parse_mcp_output_image( +fn parse_mcp_media_block( scope: &mut v8::PinScope<'_, '_>, value: v8::Local<'_, v8::Value>, -) -> Result<(String, Option), String> { - let Some(result) = v8_value_to_json(scope, value)? else { - return Err(IMAGE_HELPER_EXPECTS_MESSAGE.to_string()); + expected_type: &str, + expects_message: &str, +) -> Result { + let Some(JsonValue::Object(mut block)) = v8_value_to_json(scope, value)? else { + return Err(expects_message.to_string()); }; - let JsonValue::Object(result) = result else { - return Err(IMAGE_HELPER_EXPECTS_MESSAGE.to_string()); + let Some(item_type) = block.get("type").and_then(JsonValue::as_str) else { + return Err(expects_message.to_string()); }; - let Some(item_type) = result.get("type").and_then(JsonValue::as_str) else { - return Err(IMAGE_HELPER_EXPECTS_MESSAGE.to_string()); - }; - if item_type != "image" { + if item_type != expected_type { return Err(format!( - "image only accepts MCP image blocks, got \"{item_type}\"" + "{expected_type} only accepts MCP {expected_type} blocks, got \"{item_type}\"" )); } - let data = result + if block .get("data") .and_then(JsonValue::as_str) - .ok_or_else(|| "image expected MCP image data".to_string())?; - if data.is_empty() { - return Err("image expected MCP image data".to_string()); + .is_none_or(str::is_empty) + { + return Err(format!("{expected_type} expected MCP {expected_type} data")); } - - let image_url = if data.to_ascii_lowercase().starts_with("data:") { - data.to_string() - } else { - let mime_type = result - .get("mimeType") - .or_else(|| result.get("mime_type")) - .and_then(JsonValue::as_str) - .filter(|mime_type| !mime_type.is_empty()) - .unwrap_or("application/octet-stream"); - format!("data:{mime_type};base64,{data}") - }; - let detail = result - .get("_meta") - .and_then(JsonValue::as_object) - .and_then(|meta| meta.get(CODEX_IMAGE_DETAIL_META_KEY)) - .and_then(JsonValue::as_str) - .filter(|detail| matches!(*detail, "auto" | "low" | "high" | "original")) - .map(str::to_string); - Ok((image_url, detail)) + // image() and audio() historically prefer mimeType over its alias and treat + // an empty or non-string MIME type as absent. + if let Some(alias) = block.remove("mime_type") { + block.entry("mimeType").or_insert(alias); + } + if block + .get("mimeType") + .is_some_and(|mime| mime.as_str().is_none_or(str::is_empty)) + { + block.remove("mimeType"); + } + mcp_output_item(&JsonValue::Object(block)) } -fn parse_image_detail_value<'s>( - scope: &mut v8::PinScope<'s, '_>, - value: Option>, +fn parse_image_detail_value( + scope: &mut v8::PinScope<'_, '_>, + value: Option>, ) -> Result, String> { match value { Some(value) if value.is_string() => Ok(Some(value.to_rust_string_lossy(scope))), @@ -201,7 +216,7 @@ pub(super) fn normalize_output_audio( if let Some(audio_url) = parse_non_mcp_output_audio(scope, object)? { audio_url } else { - parse_mcp_output_audio(scope, value)? + return parse_mcp_media_block(scope, value, "audio", AUDIO_HELPER_EXPECTS_MESSAGE); } } else { return Err(AUDIO_HELPER_EXPECTS_MESSAGE.to_string()); @@ -217,15 +232,7 @@ pub(super) fn normalize_output_audio( return Err(INVALID_AUDIO_URL_ERROR.to_string()); } - // Tiny tool-generated clips cannot be encoded reliably by audio models. - if wav_duration_seconds(&audio_url).is_some_and(|duration| duration < 0.025) { - return Ok(OutputItem::InputText { - text: "Audio output omitted because the clip is shorter than 25 ms; use a longer clip." - .to_string(), - }); - } - - Ok(OutputItem::InputAudio { audio_url }) + Ok(output_audio_item(audio_url)) })(); match result { @@ -237,6 +244,18 @@ pub(super) fn normalize_output_audio( } } +fn output_audio_item(audio_url: String) -> OutputItem { + // Tiny tool-generated clips cannot be encoded reliably by audio models. + if wav_duration_seconds(&audio_url).is_some_and(|duration| duration < 0.025) { + OutputItem::InputText { + text: "Audio output omitted because the clip is shorter than 25 ms; use a longer clip." + .to_string(), + } + } else { + OutputItem::InputAudio { audio_url } + } +} + fn parse_non_mcp_output_audio( scope: &mut v8::PinScope<'_, '_>, object: v8::Local<'_, v8::Object>, @@ -255,43 +274,22 @@ fn parse_non_mcp_output_audio( Ok(Some(audio_url.to_rust_string_lossy(scope))) } -fn parse_mcp_output_audio( +fn stringify_json_value( scope: &mut v8::PinScope<'_, '_>, value: v8::Local<'_, v8::Value>, -) -> Result { - let Some(result) = v8_value_to_json(scope, value)? else { - return Err(AUDIO_HELPER_EXPECTS_MESSAGE.to_string()); - }; - let JsonValue::Object(result) = result else { - return Err(AUDIO_HELPER_EXPECTS_MESSAGE.to_string()); - }; - let Some(item_type) = result.get("type").and_then(JsonValue::as_str) else { - return Err(AUDIO_HELPER_EXPECTS_MESSAGE.to_string()); - }; - if item_type != "audio" { - return Err(format!( - "audio only accepts MCP audio blocks, got \"{item_type}\"" - )); +) -> Result, String> { + let tc = std::pin::pin!(v8::TryCatch::new(scope)); + let mut tc = tc.init(); + if let Some(stringified) = v8::json::stringify(&tc, value) { + return Ok(Some(stringified.to_rust_string_lossy(&tc))); } - let data = result - .get("data") - .and_then(JsonValue::as_str) - .ok_or_else(|| "audio expected MCP audio data".to_string())?; - if data.is_empty() { - return Err("audio expected MCP audio data".to_string()); - } - - if data.to_ascii_lowercase().starts_with("data:") { - Ok(data.to_string()) - } else { - let mime_type = result - .get("mimeType") - .or_else(|| result.get("mime_type")) - .and_then(JsonValue::as_str) - .filter(|mime_type| !mime_type.is_empty()) - .unwrap_or("application/octet-stream"); - Ok(format!("data:{mime_type};base64,{data}")) + if tc.has_caught() { + return Err(tc + .exception() + .map(|exception| value_to_error_text(&mut tc, exception)) + .unwrap_or_else(|| "unknown code mode exception".to_string())); } + Ok(None) } pub(super) fn v8_value_to_json( @@ -307,18 +305,9 @@ pub(super) fn v8_value_to_json( return Ok(None); } - let tc = std::pin::pin!(v8::TryCatch::new(scope)); - let mut tc = tc.init(); - let Some(stringified) = v8::json::stringify(&tc, value) else { - if tc.has_caught() { - return Err(tc - .exception() - .map(|exception| value_to_error_text(&mut tc, exception)) - .unwrap_or_else(|| "unknown code mode exception".to_string())); - } + let Some(json) = stringify_json_value(scope, value)? else { return Ok(None); }; - let json = stringified.to_rust_string_lossy(&tc); // V8 emits compact JSON with these ASCII key names unescaped. The object/comma // prefix and colon distinguish keys from string contents. Reject these before // Serde can reinterpret them as values and bypass its normal recursion limit. diff --git a/codex-rs/code-mode-runtime/src/service_audio_tests.rs b/codex-rs/code-mode-runtime/src/service_audio_tests.rs index f8f4fba85a..db392754fd 100644 --- a/codex-rs/code-mode-runtime/src/service_audio_tests.rs +++ b/codex-rs/code-mode-runtime/src/service_audio_tests.rs @@ -63,7 +63,8 @@ async fn audio_helper_omits_short_wav_in_all_input_forms() { ExecuteRequest { source: format!( r#"audio({audio_url:?}); audio({{audio_url:{audio_url:?}}}); -audio({{type:"audio",mimeType:"audio/wav",data:{payload:?}}});"# +const block = {{type:"audio",mimeType:"audio/wav",data:{payload:?}}}; +audio(block); content(block);"# ), yield_time_ms: None, ..execute_request("") @@ -82,7 +83,7 @@ audio({{type:"audio",mimeType:"audio/wav",data:{payload:?}}});"# RuntimeResponse::Result { code_mode_host_duration: None, cell_id: cell_id("1"), - content_items: vec![expected; 3], + content_items: vec![expected; 4], result_buffered_for: std::time::Duration::ZERO, error_text: None, }, diff --git a/codex-rs/code-mode-runtime/src/service_tests.rs b/codex-rs/code-mode-runtime/src/service_tests.rs index 8a14b2421f..b49b2093b1 100644 --- a/codex-rs/code-mode-runtime/src/service_tests.rs +++ b/codex-rs/code-mode-runtime/src/service_tests.rs @@ -1450,6 +1450,7 @@ async fn global_scope_contains_only_allowed_items() { "add_content", "as_settled", "audio", + "content", "decodeURI", "decodeURIComponent", "encodeURI", @@ -1644,13 +1645,17 @@ text(JSON.stringify(returnsUndefined)); } #[tokio::test] -async fn text_helper_serializes_objects() { +async fn text_serializes_objects_and_content_forwards_mcp_text() { let service = InProcessCodeModeSession::new(); let response = execute( &service, ExecuteRequest { - source: "text({ json: true });".to_string(), + source: r#"const block = {type: "text", text: "caption", json: true}; +text(block); +text(block.text); +content(block);"# + .to_string(), yield_time_ms: None, ..execute_request("") }, @@ -1662,9 +1667,13 @@ async fn text_helper_serializes_objects() { RuntimeResponse::Result { code_mode_host_duration: None, cell_id: cell_id("1"), - content_items: vec![FunctionCallOutputContentItem::InputText { - text: r#"{"json":true}"#.to_string(), - }], + content_items: [ + r#"{"type":"text","text":"caption","json":true}"#, + "caption", + "caption" + ] + .map(|text| FunctionCallOutputContentItem::InputText { text: text.into() }) + .into(), result_buffered_for: Duration::ZERO, error_text: None, } @@ -1672,7 +1681,7 @@ async fn text_helper_serializes_objects() { } #[tokio::test] -async fn output_helpers_surface_serialization_errors() { +async fn output_helpers_surface_input_errors() { for (source, expected_error) in [ ( "const circular = {}; circular.self = circular; text(circular);", @@ -1686,6 +1695,14 @@ async fn output_helpers_surface_serialization_errors() { "image({type: 'image', get data() { throw new Error('MCP getter failed'); }});", "MCP getter failed", ), + ( + "image({type: 'image', data: ''});", + "image expected MCP image data", + ), + ( + "image({type: 'audio'});", + r#"image only accepts MCP image blocks, got "audio""#, + ), ] { let service = InProcessCodeModeSession::new(); let response = execute( @@ -1803,17 +1820,19 @@ async fn audio_helper_rejects_non_data_urls() { } #[tokio::test] -async fn image_helper_accepts_raw_mcp_image_block_with_original_detail() { +async fn image_and_content_forward_the_same_raw_mcp_image() { let service = InProcessCodeModeSession::new(); let response = execute(&service, ExecuteRequest { source: r#" -image({ +const block = { type: "image", data: "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==", - mimeType: "image/png", + mime_type: "image/png", _meta: { "codex/imageDetail": "original" }, -}); +}; +image(block); +content(block); "# .to_string(), yield_time_ms: None, @@ -1829,13 +1848,221 @@ image({ content_items: vec![FunctionCallOutputContentItem::InputImage { image_url: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==".to_string(), detail: Some(crate::ImageDetail::Original), - }], + }; 2], result_buffered_for: Duration::ZERO, error_text: None, } ); } +#[tokio::test] +async fn media_helpers_preserve_legacy_mime_inputs_and_detail_overrides() { + let response = execute( + &InProcessCodeModeSession::new(), + ExecuteRequest { + yield_time_ms: None, + ..execute_request( + r#" +image({type: "image", data: "YQ==", mimeType: "image/png", mime_type: "image/jpeg"}); +audio({type: "audio", data: "YQ==", mimeType: 1, mime_type: "audio/wav"}); +image({type: "image", data: "DaTa:image/png;base64,YQ==", mimeType: ""}, "LOW"); +"#, + ) + }, + ) + .await; + assert_eq!( + response, + RuntimeResponse::Result { + code_mode_host_duration: None, + cell_id: cell_id("1"), + content_items: vec![ + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,YQ==".into(), + detail: Some(crate::DEFAULT_IMAGE_DETAIL), + }, + FunctionCallOutputContentItem::InputAudio { + audio_url: "data:application/octet-stream;base64,YQ==".into(), + }, + FunctionCallOutputContentItem::InputImage { + image_url: "DaTa:image/png;base64,YQ==".into(), + detail: Some(crate::ImageDetail::Low), + }, + ], + result_buffered_for: Duration::ZERO, + error_text: None, + } + ); +} + +#[tokio::test] +async fn content_helper_forwards_mixed_mcp_blocks_in_order() { + let link = serde_json::json!({ + "type": "resource_link", "name": "notes", "uri": "file:///notes", + }); + let resource = serde_json::json!({ + "type": "resource", "resource": {"uri": "file:///notes", "text": "notes"}, + }); + let blob = serde_json::json!({ + "type": "resource", "resource": {"uri": "file:///sample", "blob": "YWJj"}, + }); + let blocks = serde_json::json!([ + {"type": "text", "text": "caption", "annotations": {"priority": 0.5}}, + { + "type": "image", + "data": "aW1hZ2U=", + "mimeType": "image/png", + "_meta": {"codex/imageDetail": "original"}, + }, + {"type": "audio", "data": "YXVkaW8=", "mimeType": "audio/mpeg"}, + link, resource, blob, + ]); + let response = execute( + &InProcessCodeModeSession::new(), + ExecuteRequest { + yield_time_ms: None, + ..execute_request(&format!("content({blocks})")) + }, + ) + .await; + assert_eq!( + response, + RuntimeResponse::Result { + code_mode_host_duration: None, + cell_id: cell_id("1"), + content_items: vec![ + FunctionCallOutputContentItem::InputText { + text: "caption".into() + }, + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,aW1hZ2U=".into(), + detail: Some(crate::ImageDetail::Original), + }, + FunctionCallOutputContentItem::InputAudio { + audio_url: "data:audio/mpeg;base64,YXVkaW8=".into(), + }, + FunctionCallOutputContentItem::InputText { + text: link.to_string() + }, + FunctionCallOutputContentItem::InputText { + text: resource.to_string() + }, + FunctionCallOutputContentItem::InputText { + text: blob.to_string() + }, + ], + result_buffered_for: Duration::ZERO, + error_text: None, + } + ); +} + +#[tokio::test] +async fn content_helper_consumes_iterables_in_order_and_closes_on_error() { + let response = execute( + &InProcessCodeModeSession::new(), + ExecuteRequest { + yield_time_ms: None, + ..execute_request( + r#" +const block = (text) => ({type: "text", text}); +content([]); +[block("single")].forEach(content); +content(new Set([block("set")])); + +const failure = new Error("iterator failed"); +function* generator() { yield block("generator"); throw failure; } +try { content(generator()); } catch (error) { + if (error !== failure) throw error; + text("caught iterator error"); +} + +let reads = 0; +const iterable = { + get [Symbol.iterator]() { + if (++reads !== 1) throw new Error("iterator getter read twice"); + return function () { + if (this !== iterable) throw new Error("wrong iterator receiver"); + const items = [block("first"), {type: "image"}, block("past error")].values(); + return { + next() { text("next"); return items.next(); }, + return() { text("closed"); return {}; }, + }; + }; + }, +}; +try { content(iterable); } catch (error) { text(error); } +"#, + ) + }, + ) + .await; + let expected = [ + "single", + "set", + "generator", + "caught iterator error", + "next", + "first", + "next", + "closed", + "content expects one MCP ContentBlock", + ]; + assert_eq!( + response, + RuntimeResponse::Result { + code_mode_host_duration: None, + cell_id: cell_id("1"), + content_items: expected + .into_iter() + .map(|text| FunctionCallOutputContentItem::InputText { text: text.into() }) + .collect(), + result_buffered_for: Duration::ZERO, + error_text: None, + } + ); +} + +#[tokio::test] +async fn content_helper_rejects_invalid_blocks_and_encrypted_text_without_emitting_them() { + let response = execute( + &InProcessCodeModeSession::new(), + ExecuteRequest { + yield_time_ms: None, + ..execute_request( + r#" +for (const block of [ + undefined, "", {content: []}, [[{type: "text", text: "nested"}]], ["text", "nested"], + {type: "unknown"}, {type: "text", text: 1}, {type: "image"}, + {type: "resource", resource: {uri: "file:///notes"}}, {type: "resource_link"}, + {type: "text", text: "opaque", _meta: {"codex/encryptedContent": true}}, +]) { + try { content(block); } catch (error) { + const expected = block?._meta?.["codex/encryptedContent"] + ? "content cannot emit encrypted MCP text" + : "content expects one MCP ContentBlock"; + if (error === expected) continue; + throw error; + } + throw new Error("expected content to reject the block"); +} +"#, + ) + }, + ) + .await; + assert_eq!( + response, + RuntimeResponse::Result { + code_mode_host_duration: None, + cell_id: cell_id("1"), + content_items: Vec::new(), + result_buffered_for: Duration::ZERO, + error_text: None, + } + ); +} + #[tokio::test] async fn generated_image_helper_appends_image_and_output_hint() { let service = InProcessCodeModeSession::new(); diff --git a/codex-rs/core/src/tools/code_mode/response_adapter.rs b/codex-rs/core/src/tools/code_mode/response_adapter.rs index f28d5da934..e23ed354f6 100644 --- a/codex-rs/core/src/tools/code_mode/response_adapter.rs +++ b/codex-rs/core/src/tools/code_mode/response_adapter.rs @@ -1,7 +1,5 @@ -use codex_code_mode::ImageDetail as CodeModeImageDetail; use codex_protocol::models::DEFAULT_IMAGE_DETAIL; use codex_protocol::models::FunctionCallOutputContentItem; -use codex_protocol::models::ImageDetail; use codex_protocol::models::ImageReference; trait IntoProtocol { @@ -14,18 +12,6 @@ pub(super) fn into_function_call_output_content_items( items.into_iter().map(IntoProtocol::into_protocol).collect() } -impl IntoProtocol for CodeModeImageDetail { - fn into_protocol(self) -> ImageDetail { - let value = self; - match value { - CodeModeImageDetail::Auto => ImageDetail::Auto, - CodeModeImageDetail::Low => ImageDetail::Low, - CodeModeImageDetail::High => ImageDetail::High, - CodeModeImageDetail::Original => ImageDetail::Original, - } - } -} - impl IntoProtocol for codex_code_mode::FunctionCallOutputContentItem { @@ -38,9 +24,7 @@ impl IntoProtocol codex_code_mode::FunctionCallOutputContentItem::InputImage { image_url, detail } => { FunctionCallOutputContentItem::InputImage { image: ImageReference::Inline { image_url }, - detail: detail - .map(IntoProtocol::into_protocol) - .or(Some(DEFAULT_IMAGE_DETAIL)), + detail: detail.or(Some(DEFAULT_IMAGE_DETAIL)), } } codex_code_mode::FunctionCallOutputContentItem::InputAudio { audio_url } => { diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index 4a4de6307e..c2f01448b2 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -687,7 +687,7 @@ async fn run_code_mode_turn_with_rmcp( run_code_mode_turn_with_rmcp_model(server, prompt, code, "test-gpt-5.1-codex").await } -async fn run_code_mode_turn_with_rmcp_model( +pub(super) async fn run_code_mode_turn_with_rmcp_model( server: &MockServer, prompt: &str, code: &str, diff --git a/codex-rs/core/tests/suite/scenarios.rs b/codex-rs/core/tests/suite/scenarios.rs index 727e806a93..42a9d24ac1 100644 --- a/codex-rs/core/tests/suite/scenarios.rs +++ b/codex-rs/core/tests/suite/scenarios.rs @@ -874,6 +874,30 @@ async fn unavailable_images_are_rechecked_without_rewriting_history() -> Result< Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn code_mode_forwards_mixed_mcp_content() -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_wine_exec!(Ok(()), "requires a Windows test_stdio_server binary"); + let server = start_mock_server().await; + let (_test, response) = super::code_mode::run_code_mode_turn_with_rmcp_model( + &server, + "Show the caption and image from the MCP image tool.", + r#"const result = await tools.mcp__rmcp__image_scenario({scenario: "text_then_image", caption: "sample"}); +content(result.content);"#, + "gpt-5.5", + ) + .await?; + let output = response.single_request().custom_tool_call_output("call-1"); + assert_eq!(output["output"][1]["text"], "sample"); + assert_eq!(output["output"][2]["type"], "input_image"); + insta::assert_snapshot!(context_snapshot::format_request_history_snapshot( + "Code Mode forwards MCP text and image blocks to the model in their original order.", + &core_test_support::responses::received_responses_requests(&server).await, + &ContextSnapshotOptions::default().rewrite_known_segments(), + )); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn code_mode_mcp_schema_limits_preserve_explicit_overrides() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_forwards_mixed_mcp_content.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_forwards_mixed_mcp_content.snap new file mode 100644 index 0000000000..fc8770c311 --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__code_mode_forwards_mixed_mcp_content.snap @@ -0,0 +1,28 @@ +--- +source: core/tests/suite/scenarios.rs +expression: "context_snapshot::format_request_history_snapshot(\"Code Mode forwards MCP text and image blocks to the model in their original order.\",\n&core_test_support::responses::received_responses_requests(&server).await,\n&ContextSnapshotOptions::default().rewrite_known_segments(),)" +--- +Scenario: Code Mode forwards MCP text and image blocks to the model in their original order. + +## Window 1 +-- request 1 (turn) -- +00:message/developer: + +01:message/developer[2]: + [01] + [02] + Deferred tool namespaces: + - mcp__rmcp: Use these tools to exercise the rmcp test server. + - multi_agent_v1: Tools for spawning and managing sub-agents. + +02:message/user: + +03:message/user: + Show the caption and image from the MCP image tool. +-- request 2 (turn) -- +04:custom_tool_call/exec:const result = await tools.mcp__rmcp__image_scenario({scenario: "text_then_image", caption: "sample"}); + content(result.content); +05:custom_tool_call_output:Script completed + Wall time seconds + Output: + | sample | diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index d9f2de8178..0fed5862a7 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -37,6 +37,9 @@ mod configuration_update; mod executed_tool_calls; mod item_metadata; +mod mcp_content; + +use mcp_content::convert_mcp_content_to_items; pub use crate::local_media::MAX_PROMPT_AUDIO_INPUT_BYTES; pub use crate::local_media::snapshot_local_user_input; @@ -2576,108 +2579,6 @@ impl CallToolResult { } } -fn convert_mcp_content_to_items( - contents: &[serde_json::Value], -) -> Vec { - const CODEX_ENCRYPTED_CONTENT_META_KEY: &str = "codex/encryptedContent"; - const CODEX_IMAGE_DETAIL_META_KEY: &str = "codex/imageDetail"; - - #[derive(serde::Deserialize)] - #[serde(tag = "type")] - enum McpContent { - #[serde(rename = "text")] - Text { - text: String, - #[serde(rename = "_meta", default)] - meta: Option, - }, - #[serde(rename = "image")] - Image { - data: String, - #[serde(rename = "mimeType", alias = "mime_type")] - mime_type: Option, - #[serde(rename = "_meta", default)] - meta: Option, - }, - #[serde(rename = "audio")] - Audio { - data: String, - #[serde(rename = "mimeType", alias = "mime_type")] - mime_type: Option, - #[serde(rename = "_meta", default)] - _meta: Option, - }, - #[serde(other)] - Unknown, - } - - let mut items = Vec::with_capacity(contents.len()); - - for content in contents { - let item = match serde_json::from_value::(content.clone()) { - Ok(McpContent::Text { text, meta }) => { - if meta - .as_ref() - .and_then(|meta| meta.get(CODEX_ENCRYPTED_CONTENT_META_KEY)) - .and_then(serde_json::Value::as_bool) - == Some(true) - { - FunctionCallOutputContentItem::EncryptedContent { - encrypted_content: text, - } - } else { - FunctionCallOutputContentItem::InputText { text } - } - } - Ok(McpContent::Image { - data, - mime_type, - meta, - }) => { - let image_url = if data.starts_with("data:") { - data - } else { - let mime_type = mime_type.unwrap_or_else(|| "application/octet-stream".into()); - format!("data:{mime_type};base64,{data}") - }; - FunctionCallOutputContentItem::InputImage { - image: ImageReference::Inline { image_url }, - detail: meta - .as_ref() - .and_then(serde_json::Value::as_object) - .and_then(|meta| meta.get(CODEX_IMAGE_DETAIL_META_KEY)) - .and_then(serde_json::Value::as_str) - .and_then(|detail| match detail { - "auto" => Some(ImageDetail::Auto), - "low" => Some(ImageDetail::Low), - "high" => Some(ImageDetail::High), - "original" => Some(ImageDetail::Original), - _ => None, - }) - .or(Some(DEFAULT_IMAGE_DETAIL)), - } - } - Ok(McpContent::Audio { - data, mime_type, .. - }) => { - let audio_url = if data.starts_with("data:") { - data - } else { - let mime_type = mime_type.unwrap_or_else(|| "application/octet-stream".into()); - format!("data:{mime_type};base64,{data}") - }; - FunctionCallOutputContentItem::InputAudio { audio_url } - } - Ok(McpContent::Unknown) | Err(_) => FunctionCallOutputContentItem::InputText { - text: serde_json::to_string(content).unwrap_or_else(|_| "".to_string()), - }, - }; - items.push(item); - } - - items -} - // Implement Display so callers can treat the payload like a plain string when logging or doing // trivial substring checks in tests (existing tests call `.contains()` on the output). For // `ContentItems`, Display emits a JSON representation. @@ -3841,12 +3742,20 @@ mod tests { #[test] fn preserves_existing_image_data_urls() -> Result<()> { + let urls = [ + "data:image/png;base64,BASE64", + "DaTa:image/png;base64,BASE64", + ]; let call_tool_result = CallToolResult { - content: vec![serde_json::json!({ - "type": "image", - "data": "data:image/png;base64,BASE64", - "mimeType": "image/png" - })], + content: urls + .map(|url| { + serde_json::json!({ + "type": "image", + "data": url, + "mimeType": "image/png" + }) + }) + .into(), structured_content: None, is_error: Some(false), meta: None, @@ -3856,15 +3765,14 @@ mod tests { let Some(items) = payload.content_items() else { panic!("expected content items"); }; - let items = items.to_vec(); assert_eq!( items, - vec![FunctionCallOutputContentItem::InputImage { + &urls.map(|url| FunctionCallOutputContentItem::InputImage { image: ImageReference::Inline { - image_url: "data:image/png;base64,BASE64".into(), + image_url: url.into(), }, detail: Some(DEFAULT_IMAGE_DETAIL), - }] + }) ); Ok(()) diff --git a/codex-rs/protocol/src/models/mcp_content.rs b/codex-rs/protocol/src/models/mcp_content.rs new file mode 100644 index 0000000000..5038db4744 --- /dev/null +++ b/codex-rs/protocol/src/models/mcp_content.rs @@ -0,0 +1,104 @@ +//! Converts individual MCP content blocks into model-visible tool output. + +use super::DEFAULT_IMAGE_DETAIL; +use super::FunctionCallOutputContentItem; +use super::ImageDetail; +use super::ImageReference; +use crate::mcp::Resource; +use crate::mcp::ResourceContent; +use serde::Deserialize; +use serde_json::Value; + +#[derive(Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +enum McpContent<'a> { + Text { + text: String, + }, + Image(#[serde(borrow)] McpMedia<'a>), + Audio(#[serde(borrow)] McpMedia<'a>), + ResourceLink { + #[serde(flatten)] + _link: Resource, + }, + Resource { + #[serde(rename = "resource")] + _resource: ResourceContent, + }, +} + +#[derive(Deserialize)] +struct McpMedia<'a> { + data: &'a str, + #[serde(rename = "mimeType", alias = "mime_type")] + mime_type: Option<&'a str>, +} + +impl McpMedia<'_> { + fn data_url(&self) -> String { + if self + .data + .get(.."data:".len()) + .is_some_and(|prefix| prefix.eq_ignore_ascii_case("data:")) + { + self.data.to_owned() + } else { + let mime_type = self.mime_type.unwrap_or("application/octet-stream"); + format!("data:{mime_type};base64,{}", self.data) + } + } +} + +impl FunctionCallOutputContentItem { + /// Projects one MCP ContentBlock using its wire discriminator and Codex metadata. + pub fn from_mcp_content_block(block: &Value) -> serde_json::Result { + let meta = block.get("_meta"); + Ok(match McpContent::deserialize(block)? { + McpContent::Text { text } => { + if meta + .and_then(|meta| meta.get("codex/encryptedContent")) + .and_then(Value::as_bool) + == Some(true) + { + Self::EncryptedContent { + encrypted_content: text, + } + } else { + Self::InputText { text } + } + } + McpContent::Image(media) => Self::InputImage { + image: ImageReference::Inline { + image_url: media.data_url(), + }, + detail: Some( + meta.and_then(|meta| meta.get("codex/imageDetail")) + .filter(|detail| detail.is_string()) + .and_then(|detail| ImageDetail::deserialize(detail).ok()) + .unwrap_or(DEFAULT_IMAGE_DETAIL), + ), + }, + McpContent::Audio(media) => Self::InputAudio { + audio_url: media.data_url(), + }, + McpContent::ResourceLink { .. } | McpContent::Resource { .. } => Self::InputText { + text: serde_json::to_string(block)?, + }, + }) + } +} + +pub(super) fn convert_mcp_content_to_items( + contents: &[Value], +) -> Vec { + contents + .iter() + .map(|block| { + FunctionCallOutputContentItem::from_mcp_content_block(block).unwrap_or_else(|_| { + FunctionCallOutputContentItem::InputText { + text: serde_json::to_string(block).unwrap_or_else(|_| "".to_string()), + } + }) + }) + .collect() +}