mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: restrict spawn_agent v2 to messages (#16325)
This commit is contained in:
committed by
GitHub
Unverified
parent
4c72e62d0b
commit
285f4ea817
@@ -347,7 +347,7 @@ async fn multi_agent_v2_spawn_requires_task_name() {
|
||||
Arc::new(turn),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "inspect this repo"
|
||||
"items": [{"type": "text", "text": "inspect this repo"}]
|
||||
})),
|
||||
);
|
||||
let Err(err) = SpawnAgentHandlerV2.handle(invocation).await else {
|
||||
@@ -359,6 +359,42 @@ async fn multi_agent_v2_spawn_requires_task_name() {
|
||||
assert!(message.contains("missing field `task_name`"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_agent_v2_spawn_rejects_legacy_message_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).clone();
|
||||
config
|
||||
.features
|
||||
.enable(Feature::MultiAgentV2)
|
||||
.expect("test config should allow feature update");
|
||||
turn.config = Arc::new(config);
|
||||
|
||||
let invocation = invocation(
|
||||
Arc::new(session),
|
||||
Arc::new(turn),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "inspect this repo",
|
||||
"items": [{"type": "text", "text": "inspect this repo"}],
|
||||
"task_name": "worker"
|
||||
})),
|
||||
);
|
||||
let Err(err) = SpawnAgentHandlerV2.handle(invocation).await else {
|
||||
panic!("legacy message field should be rejected");
|
||||
};
|
||||
let FunctionCallError::RespondToModel(message) = err else {
|
||||
panic!("legacy message field should surface as a model-facing error");
|
||||
};
|
||||
assert!(message.contains("unknown field `message`"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn spawn_agent_errors_when_manager_dropped() {
|
||||
let (session, turn) = make_session_and_context().await;
|
||||
@@ -408,7 +444,7 @@ async fn multi_agent_v2_spawn_returns_path_and_send_message_accepts_relative_pat
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "inspect this repo",
|
||||
"items": [{"type": "text", "text": "inspect this repo"}],
|
||||
"task_name": "test_process"
|
||||
})),
|
||||
))
|
||||
@@ -503,7 +539,7 @@ async fn multi_agent_v2_spawn_rejects_legacy_fork_context() {
|
||||
Arc::new(turn),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "inspect this repo",
|
||||
"items": [{"type": "text", "text": "inspect this repo"}],
|
||||
"task_name": "worker",
|
||||
"fork_context": true
|
||||
})),
|
||||
@@ -542,7 +578,7 @@ async fn multi_agent_v2_spawn_rejects_invalid_fork_turns_string() {
|
||||
Arc::new(turn),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "inspect this repo",
|
||||
"items": [{"type": "text", "text": "inspect this repo"}],
|
||||
"task_name": "worker",
|
||||
"fork_turns": "banana"
|
||||
})),
|
||||
@@ -581,7 +617,7 @@ async fn multi_agent_v2_spawn_rejects_zero_fork_turns() {
|
||||
Arc::new(turn),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "inspect this repo",
|
||||
"items": [{"type": "text", "text": "inspect this repo"}],
|
||||
"task_name": "worker",
|
||||
"fork_turns": "0"
|
||||
})),
|
||||
@@ -695,7 +731,7 @@ async fn multi_agent_v2_list_agents_returns_completed_status_and_last_task_messa
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "inspect this repo",
|
||||
"items": [{"type": "text", "text": "inspect this repo"}],
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
@@ -873,7 +909,7 @@ async fn multi_agent_v2_list_agents_omits_closed_agents() {
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "inspect this repo",
|
||||
"items": [{"type": "text", "text": "inspect this repo"}],
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
@@ -937,7 +973,7 @@ async fn multi_agent_v2_send_message_rejects_structured_items() {
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "boot worker",
|
||||
"items": [{"type": "text", "text": "boot worker"}],
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
@@ -995,7 +1031,7 @@ async fn multi_agent_v2_send_message_interrupts_busy_child_without_triggering_tu
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "boot worker",
|
||||
"items": [{"type": "text", "text": "boot worker"}],
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
@@ -1132,7 +1168,7 @@ async fn multi_agent_v2_assign_task_interrupts_busy_child_without_losing_message
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "boot worker",
|
||||
"items": [{"type": "text", "text": "boot worker"}],
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
@@ -1261,7 +1297,7 @@ async fn multi_agent_v2_assign_task_completion_notifies_parent_on_every_turn() {
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "boot worker",
|
||||
"items": [{"type": "text", "text": "boot worker"}],
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
@@ -1390,7 +1426,7 @@ async fn multi_agent_v2_interrupted_turn_does_not_notify_parent() {
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "boot worker",
|
||||
"items": [{"type": "text", "text": "boot worker"}],
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
@@ -1466,7 +1502,7 @@ async fn multi_agent_v2_spawn_includes_agent_id_key_when_named() {
|
||||
Arc::new(turn),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "inspect this repo",
|
||||
"items": [{"type": "text", "text": "inspect this repo"}],
|
||||
"task_name": "test_process"
|
||||
})),
|
||||
))
|
||||
@@ -1504,7 +1540,7 @@ async fn multi_agent_v2_spawn_surfaces_task_name_validation_errors() {
|
||||
Arc::new(turn),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "inspect this repo",
|
||||
"items": [{"type": "text", "text": "inspect this repo"}],
|
||||
"task_name": "BadName"
|
||||
})),
|
||||
);
|
||||
@@ -2131,7 +2167,7 @@ async fn multi_agent_v2_wait_agent_accepts_timeout_only_argument() {
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "boot worker",
|
||||
"items": [{"type": "text", "text": "boot worker"}],
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
@@ -2377,7 +2413,7 @@ async fn multi_agent_v2_wait_agent_returns_summary_for_mailbox_activity() {
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "inspect this repo",
|
||||
"items": [{"type": "text", "text": "inspect this repo"}],
|
||||
"task_name": "test_process"
|
||||
})),
|
||||
))
|
||||
@@ -2468,7 +2504,7 @@ async fn multi_agent_v2_wait_agent_waits_for_new_mail_after_start() {
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "boot worker",
|
||||
"items": [{"type": "text", "text": "boot worker"}],
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
@@ -2568,7 +2604,7 @@ async fn multi_agent_v2_wait_agent_wakes_on_any_mailbox_notification() {
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": format!("boot {task_name}"),
|
||||
"items": [{"type": "text", "text": format!("boot {task_name}")}],
|
||||
"task_name": task_name
|
||||
})),
|
||||
))
|
||||
@@ -2655,7 +2691,7 @@ async fn multi_agent_v2_wait_agent_does_not_return_completed_content() {
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "boot worker",
|
||||
"items": [{"type": "text", "text": "boot worker"}],
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
@@ -2741,7 +2777,7 @@ async fn multi_agent_v2_close_agent_accepts_task_name_target() {
|
||||
turn.clone(),
|
||||
"spawn_agent",
|
||||
function_payload(json!({
|
||||
"message": "inspect this repo",
|
||||
"items": [{"type": "text", "text": "inspect this repo"}],
|
||||
"task_name": "worker"
|
||||
})),
|
||||
))
|
||||
|
||||
@@ -40,7 +40,7 @@ impl ToolHandler for Handler {
|
||||
.map(str::trim)
|
||||
.filter(|role| !role.is_empty());
|
||||
|
||||
let initial_operation = parse_collab_input(args.message, args.items)?;
|
||||
let initial_operation = parse_collab_input(/*message*/ None, Some(args.items))?;
|
||||
let prompt = render_input_preview(&initial_operation);
|
||||
|
||||
let session_source = turn.session_source.clone();
|
||||
@@ -200,9 +200,9 @@ impl ToolHandler for Handler {
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
struct SpawnAgentArgs {
|
||||
message: Option<String>,
|
||||
items: Option<Vec<UserInput>>,
|
||||
items: Vec<UserInput>,
|
||||
task_name: String,
|
||||
agent_type: Option<String>,
|
||||
model: Option<String>,
|
||||
|
||||
@@ -497,9 +497,14 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() {
|
||||
panic!("spawn_agent should use object params");
|
||||
};
|
||||
assert!(properties.contains_key("task_name"));
|
||||
assert!(properties.contains_key("items"));
|
||||
assert!(properties.contains_key("fork_turns"));
|
||||
assert!(!properties.contains_key("message"));
|
||||
assert!(!properties.contains_key("fork_context"));
|
||||
assert_eq!(required.as_ref(), Some(&vec!["task_name".to_string()]));
|
||||
assert_eq!(
|
||||
required.as_ref(),
|
||||
Some(&vec!["task_name".to_string(), "items".to_string()])
|
||||
);
|
||||
let output_schema = output_schema
|
||||
.as_ref()
|
||||
.expect("spawn_agent should define output schema");
|
||||
|
||||
@@ -66,7 +66,7 @@ pub fn create_spawn_agent_tool_v2(options: SpawnAgentToolOptions<'_>) -> ToolSpe
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::Object {
|
||||
properties,
|
||||
required: Some(vec!["task_name".to_string()]),
|
||||
required: Some(vec!["task_name".to_string(), "items".to_string()]),
|
||||
additional_properties: Some(false.into()),
|
||||
},
|
||||
output_schema: Some(spawn_agent_output_schema_v2()),
|
||||
@@ -594,15 +594,6 @@ fn spawn_agent_common_properties_v1(agent_type_description: &str) -> BTreeMap<St
|
||||
|
||||
fn spawn_agent_common_properties_v2(agent_type_description: &str) -> BTreeMap<String, JsonSchema> {
|
||||
BTreeMap::from([
|
||||
(
|
||||
"message".to_string(),
|
||||
JsonSchema::String {
|
||||
description: Some(
|
||||
"Initial plain-text task for the new agent. Use either message or items."
|
||||
.to_string(),
|
||||
),
|
||||
},
|
||||
),
|
||||
("items".to_string(), create_collab_input_items_schema()),
|
||||
(
|
||||
"agent_type".to_string(),
|
||||
|
||||
@@ -56,7 +56,9 @@ fn spawn_agent_tool_v2_requires_task_name_and_lists_visible_models() {
|
||||
assert!(description.contains("visible display (`visible-model`)"));
|
||||
assert!(!description.contains("hidden display (`hidden-model`)"));
|
||||
assert!(properties.contains_key("task_name"));
|
||||
assert!(properties.contains_key("items"));
|
||||
assert!(properties.contains_key("fork_turns"));
|
||||
assert!(!properties.contains_key("message"));
|
||||
assert!(!properties.contains_key("fork_context"));
|
||||
assert_eq!(
|
||||
properties.get("agent_type"),
|
||||
@@ -64,7 +66,10 @@ fn spawn_agent_tool_v2_requires_task_name_and_lists_visible_models() {
|
||||
description: Some("role help".to_string()),
|
||||
})
|
||||
);
|
||||
assert_eq!(required, Some(vec!["task_name".to_string()]));
|
||||
assert_eq!(
|
||||
required,
|
||||
Some(vec!["task_name".to_string(), "items".to_string()])
|
||||
);
|
||||
assert_eq!(
|
||||
output_schema.expect("spawn_agent output schema")["required"],
|
||||
json!(["agent_id", "task_name", "nickname"])
|
||||
|
||||
Reference in New Issue
Block a user