From b09b58ce2d52e93930be9b22c185267657d74920 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 31 Mar 2026 16:02:45 +0200 Subject: [PATCH] chore: drop interrupt from send_message (#16324) --- .../src/tools/handlers/multi_agents_tests.rs | 124 +++++------------- .../handlers/multi_agents_v2/assign_task.rs | 12 +- .../handlers/multi_agents_v2/message_tool.rs | 25 +++- .../handlers/multi_agents_v2/send_message.rs | 12 +- codex-rs/core/src/tools/spec_tests.rs | 1 + codex-rs/tools/src/agent_tool.rs | 11 +- codex-rs/tools/src/agent_tool_tests.rs | 1 + 7 files changed, 73 insertions(+), 113 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index 5531a2674..36294c49c 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -1010,7 +1010,7 @@ async fn multi_agent_v2_send_message_rejects_structured_items() { } #[tokio::test] -async fn multi_agent_v2_send_message_interrupts_busy_child_without_triggering_turn() { +async fn multi_agent_v2_send_message_rejects_interrupt_parameter() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager @@ -1043,107 +1043,43 @@ async fn multi_agent_v2_send_message_interrupts_busy_child_without_triggering_tu .resolve_agent_reference(session.conversation_id, &turn.session_source, "worker") .await .expect("worker should resolve"); - let thread = manager - .get_thread(agent_id) - .await - .expect("worker thread should exist"); - let active_turn = thread.codex.session.new_default_turn().await; - thread - .codex - .session - .spawn_task( - Arc::clone(&active_turn), - vec![UserInput::Text { - text: "working".to_string(), - text_elements: Vec::new(), - }], - NeverEndingTask, - ) - .await; + let invocation = invocation( + session, + turn, + "send_message", + function_payload(json!({ + "target": agent_id.to_string(), + "items": [{"type": "text", "text": "continue"}], + "interrupt": true + })), + ); - SendMessageHandlerV2 - .handle(invocation( - session.clone(), - turn.clone(), - "send_message", - function_payload(json!({ - "target": agent_id.to_string(), - "items": [{"type": "text", "text": "continue"}], - "interrupt": true - })), - )) - .await - .expect("interrupting v2 send_message should succeed"); + let Err(err) = SendMessageHandlerV2.handle(invocation).await else { + panic!("send_message interrupt parameter should be rejected"); + }; + let FunctionCallError::RespondToModel(message) = err else { + panic!("expected model-facing parse error"); + }; + assert!(message.starts_with( + "failed to parse function arguments: unknown field `interrupt`, expected `target` or `items`" + )); let ops = manager.captured_ops(); let ops_for_agent: Vec<&Op> = ops .iter() .filter_map(|(id, op)| (*id == agent_id).then_some(op)) .collect(); - assert!(ops_for_agent.iter().any(|op| matches!(op, Op::Interrupt))); - assert!(ops_for_agent.iter().any(|op| { - matches!( - op, - Op::InterAgentCommunication { communication } - if communication.author == AgentPath::root() - && communication.recipient.as_str() == "/root/worker" - && communication.other_recipients.is_empty() - && communication.content == "continue" - && !communication.trigger_turn - ) - })); - - timeout(Duration::from_secs(5), async { - loop { - if !thread.codex.session.has_pending_input().await { - tokio::time::sleep(Duration::from_millis(10)).await; - continue; - } - let history_items = thread - .codex - .session - .clone_history() - .await - .raw_items() - .to_vec(); - let saw_envelope = history_contains_inter_agent_communication( - &history_items, - &InterAgentCommunication::new( - AgentPath::root(), - AgentPath::try_from("/root/worker").expect("agent path"), - Vec::new(), - "continue".to_string(), - /*trigger_turn*/ false, - ), - ); - let saw_user_message = history_items.iter().any(|item| { - matches!( - item, - ResponseItem::Message { role, content, .. } - if role == "user" - && content.iter().any(|content_item| matches!( - content_item, - ContentItem::InputText { text } if text == "continue" - )) - ) - }); - if saw_envelope && !saw_user_message { - panic!("send_message should not materialize the envelope into history"); - } - if !saw_user_message { - break; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await - .expect("interrupting v2 send_message should queue the redirected message without a turn"); - - let _ = thread - .submit(Op::Shutdown {}) - .await - .expect("shutdown should submit"); + assert!(!ops_for_agent.iter().any(|op| matches!(op, Op::Interrupt))); + assert!(!ops_for_agent.iter().any(|op| matches!( + op, + Op::InterAgentCommunication { communication } + if communication.author == AgentPath::root() + && communication.recipient.as_str() == "/root/worker" + && communication.other_recipients.is_empty() + && communication.content == "continue" + && !communication.trigger_turn + ))); } #[tokio::test] diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/assign_task.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/assign_task.rs index e55b65b31..2aed22b3d 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/assign_task.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/assign_task.rs @@ -1,3 +1,4 @@ +use super::message_tool::AssignTaskArgs; use super::message_tool::MessageDeliveryMode; use super::message_tool::MessageToolResult; use super::message_tool::handle_message_tool; @@ -18,6 +19,15 @@ impl ToolHandler for Handler { } async fn handle(&self, invocation: ToolInvocation) -> Result { - handle_message_tool(invocation, MessageDeliveryMode::TriggerTurn).await + let arguments = function_arguments(invocation.payload.clone())?; + let args: AssignTaskArgs = parse_arguments(&arguments)?; + handle_message_tool( + invocation, + MessageDeliveryMode::TriggerTurn, + args.target, + args.items, + args.interrupt, + ) + .await } } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/message_tool.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/message_tool.rs index a61c01f6a..59eb49203 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/message_tool.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/message_tool.rs @@ -38,8 +38,17 @@ impl MessageDeliveryMode { } #[derive(Debug, Deserialize)] -/// Input shared by the MultiAgentV2 `send_message` and `assign_task` tools. -pub(crate) struct MessageToolArgs { +#[serde(deny_unknown_fields)] +/// Input for the MultiAgentV2 `send_message` tool. +pub(crate) struct SendMessageArgs { + pub(crate) target: String, + pub(crate) items: Vec, +} + +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +/// Input for the MultiAgentV2 `assign_task` tool. +pub(crate) struct AssignTaskArgs { pub(crate) target: String, pub(crate) items: Vec, #[serde(default)] @@ -95,6 +104,9 @@ fn text_content( pub(crate) async fn handle_message_tool( invocation: ToolInvocation, mode: MessageDeliveryMode, + target: String, + items: Vec, + interrupt: bool, ) -> Result { let ToolInvocation { session, @@ -103,16 +115,15 @@ pub(crate) async fn handle_message_tool( call_id, .. } = invocation; - let arguments = function_arguments(payload)?; - let args: MessageToolArgs = parse_arguments(&arguments)?; - let receiver_thread_id = resolve_agent_target(&session, &turn, &args.target).await?; - let prompt = text_content(&args.items, mode)?; + let _ = payload; + let receiver_thread_id = resolve_agent_target(&session, &turn, &target).await?; + let prompt = text_content(&items, mode)?; let receiver_agent = session .services .agent_control .get_agent_metadata(receiver_thread_id) .unwrap_or_default(); - if args.interrupt { + if interrupt { session .services .agent_control diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/send_message.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/send_message.rs index ed10b124a..e0e3ec910 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/send_message.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/send_message.rs @@ -1,5 +1,6 @@ use super::message_tool::MessageDeliveryMode; use super::message_tool::MessageToolResult; +use super::message_tool::SendMessageArgs; use super::message_tool::handle_message_tool; use super::*; @@ -18,6 +19,15 @@ impl ToolHandler for Handler { } async fn handle(&self, invocation: ToolInvocation) -> Result { - handle_message_tool(invocation, MessageDeliveryMode::QueueOnly).await + let arguments = function_arguments(invocation.payload.clone())?; + let args: SendMessageArgs = parse_arguments(&arguments)?; + handle_message_tool( + invocation, + MessageDeliveryMode::QueueOnly, + args.target, + args.items, + /*interrupt*/ false, + ) + .await } } diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 7c5cb8810..8bc27a072 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -526,6 +526,7 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { panic!("send_message should use object params"); }; assert!(properties.contains_key("target")); + assert!(!properties.contains_key("interrupt")); assert!(!properties.contains_key("message")); assert_eq!( required.as_ref(), diff --git a/codex-rs/tools/src/agent_tool.rs b/codex-rs/tools/src/agent_tool.rs index fc87dcc0f..84cead4f2 100644 --- a/codex-rs/tools/src/agent_tool.rs +++ b/codex-rs/tools/src/agent_tool.rs @@ -128,20 +128,11 @@ pub fn create_send_message_tool() -> ToolSpec { }, ), ("items".to_string(), create_collab_input_items_schema()), - ( - "interrupt".to_string(), - JsonSchema::Boolean { - description: Some( - "When true, stop the agent's current task and handle this immediately. When false (default), queue this message." - .to_string(), - ), - }, - ), ]); ToolSpec::Function(ResponsesApiTool { name: "send_message".to_string(), - description: "Add a message to an existing agent without triggering a new turn. Use interrupt=true to stop the current task first. In MultiAgentV2, this tool currently supports text content only." + description: "Add a message to an existing agent without triggering a new turn. In MultiAgentV2, this tool currently supports text content only." .to_string(), strict: false, defer_loading: None, diff --git a/codex-rs/tools/src/agent_tool_tests.rs b/codex-rs/tools/src/agent_tool_tests.rs index a794ff380..1a6a905a5 100644 --- a/codex-rs/tools/src/agent_tool_tests.rs +++ b/codex-rs/tools/src/agent_tool_tests.rs @@ -114,6 +114,7 @@ fn send_message_tool_requires_items_and_uses_submission_output() { }; assert!(properties.contains_key("target")); assert!(properties.contains_key("items")); + assert!(!properties.contains_key("interrupt")); assert!(!properties.contains_key("message")); assert_eq!( required,