mirror of
https://github.com/openai/codex.git
synced 2026-11-12 22:42:36 +00:00
Defer v1 multi-agent tools behind tool search (#23144)
Summary: defer v1 multi-agent tools when tool_search and namespace tools are available; keep concise searchable descriptions and move the v1 usage guidance into developer instructions; add targeted coverage. Testing: not run per request; ran just fmt.
This commit is contained in:
1 parent
80fdd4688f
commit
b3ae3de405
14 files changed
+273
-66
No files matched your search
@@ -49,13 +49,6 @@ fn developer_texts(input: &[Value]) -> Vec<String> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn developer_message_count(input: &[Value]) -> usize {
|
||||
input
|
||||
.iter()
|
||||
.filter(|item| item.get("role").and_then(Value::as_str) == Some("developer"))
|
||||
.count()
|
||||
}
|
||||
|
||||
fn collab_xml(text: &str) -> String {
|
||||
format!("{COLLABORATION_MODE_OPEN_TAG}{text}{COLLABORATION_MODE_CLOSE_TAG}")
|
||||
}
|
||||
@@ -92,7 +85,6 @@ async fn no_collaboration_instructions_by_default() -> Result<()> {
|
||||
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
let input = req.single_request().input();
|
||||
assert_eq!(developer_message_count(&input), 1);
|
||||
let dev_texts = developer_texts(&input);
|
||||
assert!(
|
||||
dev_texts
|
||||
@@ -790,7 +782,6 @@ async fn empty_collaboration_instructions_are_ignored() -> Result<()> {
|
||||
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
let input = req.single_request().input();
|
||||
assert_eq!(developer_message_count(&input), 1);
|
||||
let dev_texts = developer_texts(&input);
|
||||
let collab_text = collab_xml("");
|
||||
assert_eq!(count_messages_containing(&dev_texts, &collab_text), 0);
|
||||
|
||||
@@ -1553,7 +1553,7 @@ async fn remote_compact_trim_estimate_uses_session_base_instructions() -> Result
|
||||
let override_base_instructions = format!(
|
||||
"{}\nREMOTE_BASE_INSTRUCTIONS_OVERRIDE {}",
|
||||
baseline_compact_request.instructions_text(),
|
||||
"x".repeat(4_000)
|
||||
"x".repeat(8_000)
|
||||
);
|
||||
let override_context_window = baseline_payload_tokens.saturating_add(500);
|
||||
let pretrim_override_estimate =
|
||||
|
||||
@@ -182,11 +182,7 @@ async fn prompt_tools_are_consistent_across_requests() -> anyhow::Result<()> {
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
"view_image",
|
||||
"spawn_agent",
|
||||
"send_input",
|
||||
"resume_agent",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"tool_search",
|
||||
"web_search",
|
||||
]);
|
||||
let body0 = req1.single_request().body_json();
|
||||
|
||||
@@ -217,7 +217,7 @@ async fn always_defer_feature_hides_small_app_tool_sets() -> Result<()> {
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn search_tool_is_hidden_for_api_key_auth() -> Result<()> {
|
||||
async fn app_search_sources_are_hidden_for_api_key_auth() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
@@ -249,8 +249,13 @@ async fn search_tool_is_hidden_for_api_key_auth() -> Result<()> {
|
||||
let body = mock.single_request().body_json();
|
||||
let tools = tool_names(&body);
|
||||
assert!(
|
||||
!tools.iter().any(|name| name == TOOL_SEARCH_TOOL_NAME),
|
||||
"tools list should not include {TOOL_SEARCH_TOOL_NAME} for API key auth: {tools:?}"
|
||||
!tools.iter().any(|name| name == SEARCH_CALENDAR_NAMESPACE),
|
||||
"tools list should not include app tools for API key auth: {tools:?}"
|
||||
);
|
||||
let description = tool_search_description(&body).unwrap_or_default();
|
||||
assert!(
|
||||
!description.contains("Calendar"),
|
||||
"tool_search description should not include app sources for API key auth: {description}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
@@ -671,6 +676,96 @@ async fn tool_search_returns_deferred_tools_without_follow_up_tool_injection() -
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn tool_search_returns_deferred_v1_multi_agent_tools() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let call_id = "tool-search-spawn-agent";
|
||||
let mock = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_tool_search_call(
|
||||
call_id,
|
||||
&json!({
|
||||
"query": "spawn agent",
|
||||
"limit": 1,
|
||||
}),
|
||||
),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = test_codex().with_config(configure_search_capable_model);
|
||||
let test = builder.build(&server).await?;
|
||||
test.submit_turn_with_approval_and_permission_profile(
|
||||
"Find the spawn agent tool",
|
||||
AskForApproval::Never,
|
||||
PermissionProfile::Disabled,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let requests = mock.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
|
||||
let first_request_body = requests[0].body_json();
|
||||
let first_request_tools = tool_names(&first_request_body);
|
||||
assert!(
|
||||
first_request_tools
|
||||
.iter()
|
||||
.any(|name| name == TOOL_SEARCH_TOOL_NAME),
|
||||
"first request should advertise tool_search: {first_request_tools:?}"
|
||||
);
|
||||
for tool_name in [
|
||||
"spawn_agent",
|
||||
"send_input",
|
||||
"resume_agent",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
] {
|
||||
assert!(
|
||||
!first_request_tools.iter().any(|name| name == tool_name),
|
||||
"v1 multi-agent tools should be hidden before search: {first_request_tools:?}"
|
||||
);
|
||||
}
|
||||
assert!(
|
||||
!first_request_body
|
||||
.to_string()
|
||||
.contains("Only use `spawn_agent` if and only if"),
|
||||
"deferred v1 multi-agent guidance should stay out of initial developer context"
|
||||
);
|
||||
|
||||
let tools = tool_search_output_tools(&requests[1], call_id);
|
||||
let spawn_agent = tools
|
||||
.iter()
|
||||
.find(|tool| {
|
||||
tool.get("type").and_then(Value::as_str) == Some("function")
|
||||
&& tool.get("name").and_then(Value::as_str) == Some("spawn_agent")
|
||||
})
|
||||
.unwrap_or_else(|| panic!("expected tool_search to return spawn_agent: {tools:?}"));
|
||||
assert_eq!(
|
||||
spawn_agent.get("defer_loading").and_then(Value::as_bool),
|
||||
Some(true)
|
||||
);
|
||||
let description = spawn_agent
|
||||
.get("description")
|
||||
.and_then(Value::as_str)
|
||||
.expect("spawn_agent description should be present");
|
||||
assert!(description.contains("Only use `spawn_agent` if and only if"));
|
||||
assert!(description.contains("### Designing delegated subtasks"));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn tool_search_returns_deferred_dynamic_tool_and_routes_follow_up_call() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -9,8 +9,10 @@ use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::ev_tool_search_call;
|
||||
use core_test_support::responses::mount_response_once_match;
|
||||
use core_test_support::responses::mount_sse_once_match;
|
||||
use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::sse_response;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
@@ -18,6 +20,7 @@ use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
@@ -64,28 +67,22 @@ fn has_subagent_notification(req: &ResponsesRequest) -> bool {
|
||||
.any(|text| text.contains("<subagent_notification>"))
|
||||
}
|
||||
|
||||
fn tool_parameter_description(
|
||||
req: &ResponsesRequest,
|
||||
tool_name: &str,
|
||||
parameter_name: &str,
|
||||
) -> Option<String> {
|
||||
req.body_json()
|
||||
fn tool_parameter_description(tool: &Value, parameter_name: &str) -> Option<String> {
|
||||
tool.get("parameters")
|
||||
.and_then(|parameters| parameters.get("properties"))
|
||||
.and_then(|properties| properties.get(parameter_name))
|
||||
.and_then(|parameter| parameter.get("description"))
|
||||
.and_then(Value::as_str)
|
||||
.map(str::to_owned)
|
||||
}
|
||||
|
||||
fn tool_search_output_tools(request: &ResponsesRequest, call_id: &str) -> Vec<Value> {
|
||||
request
|
||||
.tool_search_output(call_id)
|
||||
.get("tools")
|
||||
.and_then(serde_json::Value::as_array)
|
||||
.and_then(|tools| {
|
||||
tools.iter().find_map(|tool| {
|
||||
if tool.get("name").and_then(serde_json::Value::as_str) == Some(tool_name) {
|
||||
tool.get("parameters")
|
||||
.and_then(|parameters| parameters.get("properties"))
|
||||
.and_then(|properties| properties.get(parameter_name))
|
||||
.and_then(|parameter| parameter.get("description"))
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(str::to_owned)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
})
|
||||
.and_then(Value::as_array)
|
||||
.cloned()
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
fn role_block(description: &str, role_name: &str) -> Option<String> {
|
||||
@@ -646,14 +643,27 @@ async fn spawn_agent_tool_description_mentions_role_locked_settings() -> Result<
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let resp_mock = mount_sse_once_match(
|
||||
let call_id = "tool-search-spawn-agent";
|
||||
let resp_mock = mount_sse_sequence(
|
||||
&server,
|
||||
|req: &wiremock::Request| body_contains(req, TURN_1_PROMPT),
|
||||
sse(vec![
|
||||
ev_response_created("resp-turn1-1"),
|
||||
ev_assistant_message("msg-turn1-1", "done"),
|
||||
ev_completed("resp-turn1-1"),
|
||||
]),
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-turn1-1"),
|
||||
ev_tool_search_call(
|
||||
call_id,
|
||||
&json!({
|
||||
"query": "spawn agent custom role",
|
||||
"limit": 1,
|
||||
}),
|
||||
),
|
||||
ev_completed("resp-turn1-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-turn1-2"),
|
||||
ev_assistant_message("msg-turn1-2", "done"),
|
||||
ev_completed("resp-turn1-2"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -683,8 +693,17 @@ async fn spawn_agent_tool_description_mentions_role_locked_settings() -> Result<
|
||||
|
||||
test.submit_turn(TURN_1_PROMPT).await?;
|
||||
|
||||
let request = resp_mock.single_request();
|
||||
let agent_type_description = tool_parameter_description(&request, "spawn_agent", "agent_type")
|
||||
let requests = resp_mock.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
let tools = tool_search_output_tools(&requests[1], call_id);
|
||||
let spawn_agent = tools
|
||||
.iter()
|
||||
.find(|tool| {
|
||||
tool.get("type").and_then(Value::as_str) == Some("function")
|
||||
&& tool.get("name").and_then(Value::as_str) == Some("spawn_agent")
|
||||
})
|
||||
.unwrap_or_else(|| panic!("expected tool_search to return spawn_agent: {tools:?}"));
|
||||
let agent_type_description = tool_parameter_description(spawn_agent, "agent_type")
|
||||
.expect("spawn_agent agent_type description");
|
||||
let custom_role_description =
|
||||
role_block(&agent_type_description, "custom").expect("custom role description");
|
||||
|
||||
Reference in new issue
Block a user