diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 31cadc947..d9f383ef6 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -6361,6 +6361,8 @@ use crate::memories::prompts::build_memory_tool_developer_instructions; #[cfg(test)] pub(crate) use tests::make_session_and_context; #[cfg(test)] +pub(crate) use tests::make_session_and_context_with_dynamic_tools_and_rx; +#[cfg(test)] pub(crate) use tests::make_session_and_context_with_rx; #[cfg(test)] pub(crate) use tests::make_session_configuration_for_tests; @@ -8273,9 +8275,9 @@ mod tests { (session, turn_context) } - // Like make_session_and_context, but returns Arc and the event receiver - // so tests can assert on emitted events. - pub(crate) async fn make_session_and_context_with_rx() -> ( + pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( + dynamic_tools: Vec, + ) -> ( Arc, Arc, async_channel::Receiver, @@ -8327,7 +8329,7 @@ mod tests { thread_name: None, original_config_do_not_use: Arc::clone(&config), session_source: SessionSource::Exec, - dynamic_tools: Vec::new(), + dynamic_tools, persist_extended_history: false, }; let per_turn_config = Session::build_per_turn_config(&session_configuration); @@ -8429,6 +8431,16 @@ mod tests { (session, turn_context, rx_event) } + // Like make_session_and_context, but returns Arc and the event receiver + // so tests can assert on emitted events. + pub(crate) async fn make_session_and_context_with_rx() -> ( + Arc, + Arc, + async_channel::Receiver, + ) { + make_session_and_context_with_dynamic_tools_and_rx(Vec::new()).await + } + #[tokio::test] async fn refresh_mcp_servers_is_deferred_until_next_turn() { let (session, turn_context) = make_session_and_context().await; @@ -9273,7 +9285,6 @@ mod tests { }) .to_string(), }, - source: ToolCallSource::Direct, }) .await; @@ -9313,7 +9324,6 @@ mod tests { }) .to_string(), }, - source: ToolCallSource::Direct, }) .await; @@ -9373,7 +9383,6 @@ mod tests { }) .to_string(), }, - source: ToolCallSource::Direct, }) .await; diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index 081d06687..58925622a 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -30,7 +30,6 @@ pub struct ToolInvocation { pub call_id: String, pub tool_name: String, pub payload: ToolPayload, - pub source: ToolCallSource, } #[derive(Clone, Debug)] diff --git a/codex-rs/core/src/tools/handlers/multi_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents.rs index 8fbbd0d11..d9916a300 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents.rs @@ -1004,7 +1004,6 @@ mod tests { call_id: "call-1".to_string(), tool_name: tool_name.to_string(), payload, - source: crate::tools::router::ToolCallSource::Direct, } } diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index ea05f736e..9bf03acb0 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -8,7 +8,6 @@ use tokio::fs; use crate::function_tool::FunctionCallError; use crate::protocol::EventMsg; use crate::protocol::ViewImageToolCallEvent; -use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; @@ -16,7 +15,6 @@ use crate::tools::handlers::parse_arguments; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use codex_protocol::models::ContentItem; -use codex_protocol::models::ResponseInputItem; use codex_protocol::models::local_image_content_items_with_label_number; pub struct ViewImageHandler; @@ -52,7 +50,6 @@ impl ToolHandler for ViewImageHandler { turn, payload, call_id, - source, .. } = invocation; @@ -85,24 +82,6 @@ impl ToolHandler for ViewImageHandler { let event_path = abs_path.clone(); let content = local_image_content_items_with_label_number(&abs_path, None); - if source == ToolCallSource::JsRepl - && content - .iter() - .any(|item| matches!(item, ContentItem::InputImage { .. })) - { - let input_item = ResponseInputItem::Message { - role: "user".to_string(), - content: content.clone(), - }; - if session - .inject_response_items(vec![input_item]) - .await - .is_err() - { - tracing::warn!("view_image could not find an active turn to attach image input"); - } - } - let content = content .into_iter() .map(|item| match item { diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index a5bd81049..18695ab88 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -9,6 +9,9 @@ use std::sync::Arc; use std::time::Duration; use codex_protocol::ThreadId; +use codex_protocol::models::ContentItem; +use codex_protocol::models::FunctionCallOutputContentItem; +use codex_protocol::models::ResponseInputItem; use serde::Deserialize; use serde::Serialize; use serde_json::Value as JsonValue; @@ -1050,36 +1053,78 @@ impl JsReplManager { } }; + let tool_name = req.tool_name.clone(); let call = crate::tools::router::ToolCall { - tool_name: req.tool_name, + tool_name: tool_name.clone(), call_id: req.id.clone(), payload, }; + let session = Arc::clone(&exec.session); + let turn = Arc::clone(&exec.turn); + let tracker = Arc::clone(&exec.tracker); + match router .dispatch_tool_call( - exec.session, - exec.turn, - exec.tracker, + session.clone(), + turn, + tracker, call, crate::tools::router::ToolCallSource::JsRepl, ) .await { - Ok(response) => match serde_json::to_value(response) { - Ok(value) => RunToolResult { - id: req.id, - ok: true, - response: Some(value), - error: None, - }, - Err(err) => RunToolResult { - id: req.id, - ok: false, - response: None, - error: Some(format!("failed to serialize tool output: {err}")), - }, - }, + Ok(response) => { + if let ResponseInputItem::FunctionCallOutput { output, .. } = &response + && let Some(items) = output.content_items() + { + let mut has_image = false; + let mut content = Vec::with_capacity(items.len()); + for item in items { + match item { + FunctionCallOutputContentItem::InputText { text } => { + content.push(ContentItem::InputText { text: text.clone() }); + } + FunctionCallOutputContentItem::InputImage { image_url } => { + has_image = true; + content.push(ContentItem::InputImage { + image_url: image_url.clone(), + }); + } + } + } + + if has_image + && session + .inject_response_items(vec![ResponseInputItem::Message { + role: "user".to_string(), + content, + }]) + .await + .is_err() + { + warn!( + tool_name = %tool_name, + "js_repl tool call returned image content but there was no active turn to attach it to" + ); + } + } + + match serde_json::to_value(response) { + Ok(value) => RunToolResult { + id: req.id, + ok: true, + response: Some(value), + error: None, + }, + Err(err) => RunToolResult { + id: req.id, + ok: false, + response: None, + error: Some(format!("failed to serialize tool output: {err}")), + }, + } + } Err(err) => RunToolResult { id: req.id, ok: false, @@ -1301,9 +1346,14 @@ pub(crate) fn resolve_node(config_path: Option<&Path>) -> Option { mod tests { use super::*; use crate::codex::make_session_and_context; + use crate::codex::make_session_and_context_with_dynamic_tools_and_rx; use crate::protocol::AskForApproval; + use crate::protocol::EventMsg; use crate::protocol::SandboxPolicy; use crate::turn_diff_tracker::TurnDiffTracker; + use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem; + use codex_protocol::dynamic_tools::DynamicToolResponse; + use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseInputItem; use codex_protocol::openai_models::InputModality; @@ -2116,23 +2166,118 @@ console.log(out.output?.body?.text ?? ""); assert!(result.output.contains("function_call_output")); let pending_input = session.get_pending_input().await; - let image_url = pending_input - .iter() - .find_map(|item| match item { - ResponseInputItem::Message { content, .. } => { - content.iter().find_map(|content_item| match content_item { - ContentItem::InputImage { image_url } => Some(image_url.as_str()), - _ => None, - }) - } - _ => None, - }) - .expect("view_image should inject an input_image message for the active turn"); + let [ResponseInputItem::Message { role, content }] = pending_input.as_slice() else { + panic!( + "view_image should inject exactly one pending input message, got {pending_input:?}" + ); + }; + assert_eq!(role, "user"); + let [ContentItem::InputImage { image_url }] = content.as_slice() else { + panic!( + "view_image should inject exactly one input_image content item, got {content:?}" + ); + }; assert!(image_url.starts_with("data:image/png;base64,")); Ok(()) } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn js_repl_can_attach_image_via_dynamic_tool_with_mixed_content() -> anyhow::Result<()> { + if !can_run_js_repl_runtime_tests().await { + return Ok(()); + } + + let (session, turn, rx_event) = + make_session_and_context_with_dynamic_tools_and_rx(vec![DynamicToolSpec { + name: "inline_image".to_string(), + description: "Returns inline text and image content.".to_string(), + input_schema: serde_json::json!({ + "type": "object", + "properties": {}, + "additionalProperties": false + }), + }]) + .await; + if !turn + .model_info + .input_modalities + .contains(&InputModality::Image) + { + return Ok(()); + } + + *session.active_turn.lock().await = Some(crate::state::ActiveTurn::default()); + + let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default())); + let manager = turn.js_repl.manager().await?; + let code = r#" +const out = await codex.tool("inline_image", {}); +console.log(out.type); +"#; + let image_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg=="; + + let session_for_response = Arc::clone(&session); + let response_watcher = async move { + loop { + let event = tokio::time::timeout(Duration::from_secs(2), rx_event.recv()).await??; + if let EventMsg::DynamicToolCallRequest(request) = event.msg { + session_for_response + .notify_dynamic_tool_response( + &request.call_id, + DynamicToolResponse { + content_items: vec![ + DynamicToolCallOutputContentItem::InputText { + text: "inline image note".to_string(), + }, + DynamicToolCallOutputContentItem::InputImage { + image_url: image_url.to_string(), + }, + ], + success: true, + }, + ) + .await; + return Ok::<(), anyhow::Error>(()); + } + } + }; + + let (result, response_watcher_result) = tokio::join!( + manager.execute( + Arc::clone(&session), + Arc::clone(&turn), + tracker, + JsReplArgs { + code: code.to_string(), + timeout_ms: Some(15_000), + }, + ), + response_watcher, + ); + response_watcher_result?; + let result = result?; + assert!(result.output.contains("function_call_output")); + + let pending_input = session.get_pending_input().await; + assert_eq!( + pending_input, + vec![ResponseInputItem::Message { + role: "user".to_string(), + content: vec![ + ContentItem::InputText { + text: "inline image note".to_string(), + }, + ContentItem::InputImage { + image_url: image_url.to_string(), + }, + ], + }] + ); + + Ok(()) + } + #[tokio::test] async fn js_repl_does_not_expose_process_global() -> anyhow::Result<()> { if !can_run_js_repl_runtime_tests().await { diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index d95a2c49a..4897b4ea6 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -175,7 +175,6 @@ impl ToolRouter { call_id, tool_name, payload, - source, }; match self.registry.dispatch(invocation).await { diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 5d2c8e3d5..d3b719550 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -36,28 +36,37 @@ use image::GenericImageView; use image::ImageBuffer; use image::Rgba; use image::load_from_memory; +use pretty_assertions::assert_eq; use serde_json::Value; use tokio::time::Duration; use wiremock::BodyPrintLimit; use wiremock::MockServer; -fn find_image_message(body: &Value) -> Option<&Value> { +fn image_messages(body: &Value) -> Vec<&Value> { body.get("input") .and_then(Value::as_array) - .and_then(|items| { - items.iter().find(|item| { - item.get("type").and_then(Value::as_str) == Some("message") - && item - .get("content") - .and_then(Value::as_array) - .map(|content| { - content.iter().any(|span| { - span.get("type").and_then(Value::as_str) == Some("input_image") + .map(|items| { + items + .iter() + .filter(|item| { + item.get("type").and_then(Value::as_str) == Some("message") + && item + .get("content") + .and_then(Value::as_array) + .map(|content| { + content.iter().any(|span| { + span.get("type").and_then(Value::as_str) == Some("input_image") + }) }) - }) - .unwrap_or(false) - }) + .unwrap_or(false) + }) + .collect() }) + .unwrap_or_default() +} + +fn find_image_message(body: &Value) -> Option<&Value> { + image_messages(body).into_iter().next() } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -366,8 +375,16 @@ console.log(out.output?.body?.text ?? ""); ); let body = req.body_json(); - let image_message = - find_image_message(&body).expect("pending input image message not included in request"); + let image_messages = image_messages(&body); + assert_eq!( + image_messages.len(), + 1, + "js_repl view_image should inject exactly one pending input image message" + ); + let image_message = image_messages + .into_iter() + .next() + .expect("pending input image message not included in request"); let image_url = image_message .get("content") .and_then(Value::as_array)