mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Use message string in v2 send_message (#16409)
## Summary - switch MultiAgentV2 send_message to accept a single message string instead of items - keep the old assign_task item parser in place for the next branch - update send_message schema/spec and focused handler tests ## Verification - cargo test -p codex-tools send_message_tool_requires_message_and_uses_submission_output - cargo test -p codex-core multi_agent_v2_send_message - just fix -p codex-tools - just fix -p codex-core - just argument-comment-lint --------- Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
d0474f2bc1
commit
23d638a573
@@ -496,7 +496,7 @@ async fn multi_agent_v2_spawn_returns_path_and_send_message_accepts_relative_pat
|
||||
"send_message",
|
||||
function_payload(json!({
|
||||
"target": "test_process",
|
||||
"items": [{"type": "text", "text": "continue"}]
|
||||
"message": "continue"
|
||||
})),
|
||||
))
|
||||
.await
|
||||
@@ -689,7 +689,7 @@ async fn multi_agent_v2_send_message_accepts_root_target_from_child() {
|
||||
"send_message",
|
||||
function_payload(json!({
|
||||
"target": "/root",
|
||||
"items": [{"type": "text", "text": "done"}]
|
||||
"message": "done"
|
||||
})),
|
||||
))
|
||||
.await
|
||||
@@ -952,7 +952,7 @@ async fn multi_agent_v2_list_agents_omits_closed_agents() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_agent_v2_send_message_rejects_structured_items() {
|
||||
async fn multi_agent_v2_send_message_rejects_legacy_items_field() {
|
||||
let (mut session, mut turn) = make_session_and_context().await;
|
||||
let manager = thread_manager();
|
||||
let root = manager
|
||||
@@ -999,14 +999,12 @@ async fn multi_agent_v2_send_message_rejects_structured_items() {
|
||||
);
|
||||
|
||||
let Err(err) = SendMessageHandlerV2.handle(invocation).await else {
|
||||
panic!("structured items should be rejected in v2");
|
||||
panic!("legacy items field should be rejected in v2");
|
||||
};
|
||||
assert_eq!(
|
||||
err,
|
||||
FunctionCallError::RespondToModel(
|
||||
"send_message only supports text content in MultiAgentV2 for now".to_string()
|
||||
)
|
||||
);
|
||||
let FunctionCallError::RespondToModel(message) = err else {
|
||||
panic!("legacy items field should surface as a model-facing error");
|
||||
};
|
||||
assert!(message.contains("unknown field `items`"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -1050,7 +1048,7 @@ async fn multi_agent_v2_send_message_rejects_interrupt_parameter() {
|
||||
"send_message",
|
||||
function_payload(json!({
|
||||
"target": agent_id.to_string(),
|
||||
"items": [{"type": "text", "text": "continue"}],
|
||||
"message": "continue",
|
||||
"interrupt": true
|
||||
})),
|
||||
);
|
||||
@@ -1062,7 +1060,7 @@ async fn multi_agent_v2_send_message_rejects_interrupt_parameter() {
|
||||
panic!("expected model-facing parse error");
|
||||
};
|
||||
assert!(message.starts_with(
|
||||
"failed to parse function arguments: unknown field `interrupt`, expected `target` or `items`"
|
||||
"failed to parse function arguments: unknown field `interrupt`, expected `target` or `message`"
|
||||
));
|
||||
|
||||
let ops = manager.captured_ops();
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
//! Shared argument parsing and dispatch for the v2 text-only agent messaging tools.
|
||||
//!
|
||||
//! `send_message` and `assign_task` intentionally expose the same input shape and differ only in
|
||||
//! whether the resulting `InterAgentCommunication` should wake the target immediately.
|
||||
//! `send_message` and `assign_task` share the same submission path and differ only in whether the
|
||||
//! resulting `InterAgentCommunication` should wake the target immediately.
|
||||
|
||||
use super::*;
|
||||
use crate::agent::control::render_input_preview;
|
||||
@@ -42,7 +42,7 @@ impl MessageDeliveryMode {
|
||||
/// Input for the MultiAgentV2 `send_message` tool.
|
||||
pub(crate) struct SendMessageArgs {
|
||||
pub(crate) target: String,
|
||||
pub(crate) items: Vec<UserInput>,
|
||||
pub(crate) message: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
@@ -79,6 +79,15 @@ impl ToolOutput for MessageToolResult {
|
||||
}
|
||||
}
|
||||
|
||||
fn message_content(message: String) -> Result<String, FunctionCallError> {
|
||||
if message.trim().is_empty() {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"Empty message can't be sent to an agent".to_string(),
|
||||
));
|
||||
}
|
||||
Ok(message)
|
||||
}
|
||||
|
||||
/// Validates that the tool input is non-empty text-only content and returns its preview string.
|
||||
fn text_content(
|
||||
items: &[UserInput],
|
||||
@@ -100,6 +109,24 @@ fn text_content(
|
||||
))
|
||||
}
|
||||
|
||||
/// Handles the shared MultiAgentV2 text-message flow for both `send_message` and `assign_task`.
|
||||
pub(crate) async fn handle_message_string_tool(
|
||||
invocation: ToolInvocation,
|
||||
mode: MessageDeliveryMode,
|
||||
target: String,
|
||||
message: String,
|
||||
interrupt: bool,
|
||||
) -> Result<MessageToolResult, FunctionCallError> {
|
||||
handle_message_submission(
|
||||
invocation,
|
||||
mode,
|
||||
target,
|
||||
message_content(message)?,
|
||||
interrupt,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Handles the shared MultiAgentV2 text-message flow for both `send_message` and `assign_task`.
|
||||
pub(crate) async fn handle_message_tool(
|
||||
invocation: ToolInvocation,
|
||||
@@ -107,6 +134,23 @@ pub(crate) async fn handle_message_tool(
|
||||
target: String,
|
||||
items: Vec<UserInput>,
|
||||
interrupt: bool,
|
||||
) -> Result<MessageToolResult, FunctionCallError> {
|
||||
handle_message_submission(
|
||||
invocation,
|
||||
mode,
|
||||
target,
|
||||
text_content(&items, mode)?,
|
||||
interrupt,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn handle_message_submission(
|
||||
invocation: ToolInvocation,
|
||||
mode: MessageDeliveryMode,
|
||||
target: String,
|
||||
prompt: String,
|
||||
interrupt: bool,
|
||||
) -> Result<MessageToolResult, FunctionCallError> {
|
||||
let ToolInvocation {
|
||||
session,
|
||||
@@ -117,7 +161,6 @@ pub(crate) async fn handle_message_tool(
|
||||
} = invocation;
|
||||
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
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
use super::message_tool::MessageDeliveryMode;
|
||||
use super::message_tool::MessageToolResult;
|
||||
use super::message_tool::SendMessageArgs;
|
||||
use super::message_tool::handle_message_tool;
|
||||
use super::message_tool::handle_message_string_tool;
|
||||
use super::*;
|
||||
|
||||
pub(crate) struct Handler;
|
||||
@@ -21,11 +21,11 @@ impl ToolHandler for Handler {
|
||||
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
|
||||
let arguments = function_arguments(invocation.payload.clone())?;
|
||||
let args: SendMessageArgs = parse_arguments(&arguments)?;
|
||||
handle_message_tool(
|
||||
handle_message_string_tool(
|
||||
invocation,
|
||||
MessageDeliveryMode::QueueOnly,
|
||||
args.target,
|
||||
args.items,
|
||||
args.message,
|
||||
/*interrupt*/ false,
|
||||
)
|
||||
.await
|
||||
|
||||
@@ -527,10 +527,11 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() {
|
||||
};
|
||||
assert!(properties.contains_key("target"));
|
||||
assert!(!properties.contains_key("interrupt"));
|
||||
assert!(!properties.contains_key("message"));
|
||||
assert!(properties.contains_key("message"));
|
||||
assert!(!properties.contains_key("items"));
|
||||
assert_eq!(
|
||||
required.as_ref(),
|
||||
Some(&vec!["target".to_string(), "items".to_string()])
|
||||
Some(&vec!["target".to_string(), "message".to_string()])
|
||||
);
|
||||
|
||||
let assign_task = find_tool(&tools, "assign_task");
|
||||
|
||||
@@ -127,7 +127,12 @@ pub fn create_send_message_tool() -> ToolSpec {
|
||||
),
|
||||
},
|
||||
),
|
||||
("items".to_string(), create_collab_input_items_schema()),
|
||||
(
|
||||
"message".to_string(),
|
||||
JsonSchema::String {
|
||||
description: Some("Message text to queue on the target agent.".to_string()),
|
||||
},
|
||||
),
|
||||
]);
|
||||
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
@@ -138,7 +143,7 @@ pub fn create_send_message_tool() -> ToolSpec {
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::Object {
|
||||
properties,
|
||||
required: Some(vec!["target".to_string(), "items".to_string()]),
|
||||
required: Some(vec!["target".to_string(), "message".to_string()]),
|
||||
additional_properties: Some(false.into()),
|
||||
},
|
||||
output_schema: Some(send_input_output_schema()),
|
||||
|
||||
@@ -95,7 +95,7 @@ fn spawn_agent_tool_v1_keeps_legacy_fork_context_field() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn send_message_tool_requires_items_and_uses_submission_output() {
|
||||
fn send_message_tool_requires_message_and_uses_submission_output() {
|
||||
let ToolSpec::Function(ResponsesApiTool {
|
||||
parameters,
|
||||
output_schema,
|
||||
@@ -113,12 +113,12 @@ fn send_message_tool_requires_items_and_uses_submission_output() {
|
||||
panic!("send_message should use object params");
|
||||
};
|
||||
assert!(properties.contains_key("target"));
|
||||
assert!(properties.contains_key("items"));
|
||||
assert!(properties.contains_key("message"));
|
||||
assert!(!properties.contains_key("interrupt"));
|
||||
assert!(!properties.contains_key("message"));
|
||||
assert!(!properties.contains_key("items"));
|
||||
assert_eq!(
|
||||
required,
|
||||
Some(vec!["target".to_string(), "items".to_string()])
|
||||
Some(vec!["target".to_string(), "message".to_string()])
|
||||
);
|
||||
assert_eq!(
|
||||
output_schema.expect("send_message output schema")["required"],
|
||||
|
||||
Reference in New Issue
Block a user