mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Use message string in v2 assign_task (#16419)
Fix assign task and clean everything --------- Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
23d638a573
commit
3152d1a557
@@ -1140,7 +1140,7 @@ async fn multi_agent_v2_assign_task_interrupts_busy_child_without_losing_message
|
||||
"assign_task",
|
||||
function_payload(json!({
|
||||
"target": agent_id.to_string(),
|
||||
"items": [{"type": "text", "text": "continue"}],
|
||||
"message": "continue",
|
||||
"interrupt": true
|
||||
})),
|
||||
))
|
||||
@@ -1269,7 +1269,7 @@ async fn multi_agent_v2_assign_task_completion_notifies_parent_on_every_turn() {
|
||||
"assign_task",
|
||||
function_payload(json!({
|
||||
"target": agent_id.to_string(),
|
||||
"items": [{"type": "text", "text": "continue"}],
|
||||
"message": "continue",
|
||||
})),
|
||||
))
|
||||
.await
|
||||
@@ -1338,6 +1338,59 @@ async fn multi_agent_v2_assign_task_completion_notifies_parent_on_every_turn() {
|
||||
assert_eq!(notifications.len(), 2);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_agent_v2_assign_task_rejects_legacy_items_field() {
|
||||
let (mut session, mut turn) = make_session_and_context().await;
|
||||
let manager = thread_manager();
|
||||
let root = manager
|
||||
.start_thread((*turn.config).clone())
|
||||
.await
|
||||
.expect("root thread should start");
|
||||
session.services.agent_control = manager.agent_control();
|
||||
session.conversation_id = root.thread_id;
|
||||
let mut config = turn.config.as_ref().clone();
|
||||
let _ = config.features.enable(Feature::MultiAgentV2);
|
||||
turn.config = Arc::new(config);
|
||||
let session = Arc::new(session);
|
||||
let turn = Arc::new(turn);
|
||||
|
||||
SpawnAgentHandlerV2
|
||||
.handle(invocation(
|
||||
session.clone(),
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "boot worker",
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
.await
|
||||
.expect("spawn worker");
|
||||
let agent_id = session
|
||||
.services
|
||||
.agent_control
|
||||
.resolve_agent_reference(session.conversation_id, &turn.session_source, "worker")
|
||||
.await
|
||||
.expect("worker should resolve");
|
||||
let invocation = invocation(
|
||||
session,
|
||||
turn,
|
||||
"assign_task",
|
||||
function_payload(json!({
|
||||
"target": agent_id.to_string(),
|
||||
"items": [{"type": "text", "text": "continue"}],
|
||||
})),
|
||||
);
|
||||
|
||||
let Err(err) = AssignTaskHandlerV2.handle(invocation).await else {
|
||||
panic!("legacy items field should be rejected in v2");
|
||||
};
|
||||
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]
|
||||
async fn multi_agent_v2_interrupted_turn_does_not_notify_parent() {
|
||||
let (mut session, mut turn) = make_session_and_context().await;
|
||||
@@ -1360,7 +1413,7 @@ async fn multi_agent_v2_interrupted_turn_does_not_notify_parent() {
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"items": [{"type": "text", "text": "boot worker"}],
|
||||
"message": "boot worker",
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
use super::message_tool::AssignTaskArgs;
|
||||
use super::message_tool::MessageDeliveryMode;
|
||||
use super::message_tool::MessageToolResult;
|
||||
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: AssignTaskArgs = parse_arguments(&arguments)?;
|
||||
handle_message_tool(
|
||||
handle_message_string_tool(
|
||||
invocation,
|
||||
MessageDeliveryMode::TriggerTurn,
|
||||
args.target,
|
||||
args.items,
|
||||
args.message,
|
||||
args.interrupt,
|
||||
)
|
||||
.await
|
||||
|
||||
@@ -4,7 +4,6 @@
|
||||
//! resulting `InterAgentCommunication` should wake the target immediately.
|
||||
|
||||
use super::*;
|
||||
use crate::agent::control::render_input_preview;
|
||||
use codex_protocol::protocol::InterAgentCommunication;
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
@@ -14,14 +13,6 @@ pub(crate) enum MessageDeliveryMode {
|
||||
}
|
||||
|
||||
impl MessageDeliveryMode {
|
||||
/// Returns the model-visible error message for non-text inputs.
|
||||
fn unsupported_items_error(self) -> &'static str {
|
||||
match self {
|
||||
Self::QueueOnly => "send_message only supports text content in MultiAgentV2 for now",
|
||||
Self::TriggerTurn => "assign_task only supports text content in MultiAgentV2 for now",
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns whether the produced communication should start a turn immediately.
|
||||
fn apply(self, communication: InterAgentCommunication) -> InterAgentCommunication {
|
||||
match self {
|
||||
@@ -50,7 +41,7 @@ pub(crate) struct SendMessageArgs {
|
||||
/// Input for the MultiAgentV2 `assign_task` tool.
|
||||
pub(crate) struct AssignTaskArgs {
|
||||
pub(crate) target: String,
|
||||
pub(crate) items: Vec<UserInput>,
|
||||
pub(crate) message: String,
|
||||
#[serde(default)]
|
||||
pub(crate) interrupt: bool,
|
||||
}
|
||||
@@ -88,28 +79,7 @@ fn message_content(message: String) -> Result<String, FunctionCallError> {
|
||||
Ok(message)
|
||||
}
|
||||
|
||||
/// Validates that the tool input is non-empty text-only content and returns its preview string.
|
||||
fn text_content(
|
||||
items: &[UserInput],
|
||||
mode: MessageDeliveryMode,
|
||||
) -> Result<String, FunctionCallError> {
|
||||
if items.is_empty() {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"Items can't be empty".to_string(),
|
||||
));
|
||||
}
|
||||
if items
|
||||
.iter()
|
||||
.all(|item| matches!(item, UserInput::Text { .. }))
|
||||
{
|
||||
return Ok(render_input_preview(&(items.to_vec().into())));
|
||||
}
|
||||
Err(FunctionCallError::RespondToModel(
|
||||
mode.unsupported_items_error().to_string(),
|
||||
))
|
||||
}
|
||||
|
||||
/// Handles the shared MultiAgentV2 text-message flow for both `send_message` and `assign_task`.
|
||||
/// Handles the shared MultiAgentV2 plain-text message flow for both `send_message` and `assign_task`.
|
||||
pub(crate) async fn handle_message_string_tool(
|
||||
invocation: ToolInvocation,
|
||||
mode: MessageDeliveryMode,
|
||||
@@ -127,24 +97,6 @@ pub(crate) async fn handle_message_string_tool(
|
||||
.await
|
||||
}
|
||||
|
||||
/// Handles the shared MultiAgentV2 text-message flow for both `send_message` and `assign_task`.
|
||||
pub(crate) async fn handle_message_tool(
|
||||
invocation: ToolInvocation,
|
||||
mode: MessageDeliveryMode,
|
||||
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,
|
||||
|
||||
@@ -547,10 +547,11 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() {
|
||||
panic!("assign_task should use object params");
|
||||
};
|
||||
assert!(properties.contains_key("target"));
|
||||
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 wait_agent = find_tool(&tools, "wait_agent");
|
||||
|
||||
@@ -160,7 +160,12 @@ pub fn create_assign_task_tool() -> ToolSpec {
|
||||
),
|
||||
},
|
||||
),
|
||||
("items".to_string(), create_collab_input_items_schema()),
|
||||
(
|
||||
"message".to_string(),
|
||||
JsonSchema::String {
|
||||
description: Some("Message text to send to the target agent.".to_string()),
|
||||
},
|
||||
),
|
||||
(
|
||||
"interrupt".to_string(),
|
||||
JsonSchema::Boolean {
|
||||
@@ -180,7 +185,7 @@ pub fn create_assign_task_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()),
|
||||
|
||||
@@ -126,6 +126,38 @@ fn send_message_tool_requires_message_and_uses_submission_output() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn assign_task_tool_requires_message_and_uses_submission_output() {
|
||||
let ToolSpec::Function(ResponsesApiTool {
|
||||
parameters,
|
||||
output_schema,
|
||||
..
|
||||
}) = create_assign_task_tool()
|
||||
else {
|
||||
panic!("assign_task should be a function tool");
|
||||
};
|
||||
let JsonSchema::Object {
|
||||
properties,
|
||||
required,
|
||||
..
|
||||
} = parameters
|
||||
else {
|
||||
panic!("assign_task should use object params");
|
||||
};
|
||||
assert!(properties.contains_key("target"));
|
||||
assert!(properties.contains_key("message"));
|
||||
assert!(properties.contains_key("interrupt"));
|
||||
assert!(!properties.contains_key("items"));
|
||||
assert_eq!(
|
||||
required,
|
||||
Some(vec!["target".to_string(), "message".to_string()])
|
||||
);
|
||||
assert_eq!(
|
||||
output_schema.expect("assign_task output schema")["required"],
|
||||
json!(["submission_id"])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn wait_agent_tool_v2_uses_timeout_only_summary_output() {
|
||||
let ToolSpec::Function(ResponsesApiTool {
|
||||
|
||||
Reference in New Issue
Block a user