mirror of
https://github.com/openai/codex.git
synced 2026-11-13 22:47:20 +00:00
chore: keep request_user_input tool to persist cache on multi-agents (#17009)
This commit is contained in:
1 parent
413c1e1fdf
commit
4cc6818996
6 files changed
+82
-10
No files matched your search
@@ -5,6 +5,7 @@ use crate::tools::context::ToolPayload;
|
|||||||
use crate::tools::handlers::parse_arguments;
|
use crate::tools::handlers::parse_arguments;
|
||||||
use crate::tools::registry::ToolHandler;
|
use crate::tools::registry::ToolHandler;
|
||||||
use crate::tools::registry::ToolKind;
|
use crate::tools::registry::ToolKind;
|
||||||
|
use codex_protocol::protocol::SessionSource;
|
||||||
use codex_protocol::request_user_input::RequestUserInputArgs;
|
use codex_protocol::request_user_input::RequestUserInputArgs;
|
||||||
use codex_tools::REQUEST_USER_INPUT_TOOL_NAME;
|
use codex_tools::REQUEST_USER_INPUT_TOOL_NAME;
|
||||||
use codex_tools::normalize_request_user_input_args;
|
use codex_tools::normalize_request_user_input_args;
|
||||||
@@ -39,6 +40,12 @@ impl ToolHandler for RequestUserInputHandler {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
if matches!(turn.session_source, SessionSource::SubAgent(_)) {
|
||||||
|
return Err(FunctionCallError::RespondToModel(
|
||||||
|
"request_user_input can only be used by the root thread".to_string(),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
let mode = session.collaboration_mode().await.mode;
|
let mode = session.collaboration_mode().await.mode;
|
||||||
if let Some(message) =
|
if let Some(message) =
|
||||||
request_user_input_unavailable_message(mode, self.default_mode_request_user_input)
|
request_user_input_unavailable_message(mode, self.default_mode_request_user_input)
|
||||||
@@ -67,3 +74,7 @@ impl ToolHandler for RequestUserInputHandler {
|
|||||||
Ok(FunctionToolOutput::from_text(content, Some(true)))
|
Ok(FunctionToolOutput::from_text(content, Some(true)))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
#[path = "request_user_input_tests.rs"]
|
||||||
|
mod tests;
|
||||||
@@ -0,0 +1,66 @@
|
|||||||
|
use super::*;
|
||||||
|
use crate::codex::make_session_and_context;
|
||||||
|
use crate::tools::context::ToolInvocation;
|
||||||
|
use crate::tools::context::ToolPayload;
|
||||||
|
use crate::turn_diff_tracker::TurnDiffTracker;
|
||||||
|
use codex_protocol::ThreadId;
|
||||||
|
use codex_protocol::protocol::SubAgentSource;
|
||||||
|
use pretty_assertions::assert_eq;
|
||||||
|
use serde_json::json;
|
||||||
|
use std::sync::Arc;
|
||||||
|
use tokio::sync::Mutex;
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn multi_agent_v2_request_user_input_rejects_subagent_threads() {
|
||||||
|
let (session, mut turn) = make_session_and_context().await;
|
||||||
|
turn.session_source = SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||||
|
parent_thread_id: ThreadId::new(),
|
||||||
|
depth: 1,
|
||||||
|
agent_path: None,
|
||||||
|
agent_nickname: None,
|
||||||
|
agent_role: None,
|
||||||
|
});
|
||||||
|
|
||||||
|
let result = RequestUserInputHandler {
|
||||||
|
default_mode_request_user_input: true,
|
||||||
|
}
|
||||||
|
.handle(ToolInvocation {
|
||||||
|
session: Arc::new(session),
|
||||||
|
turn: Arc::new(turn),
|
||||||
|
tracker: Arc::new(Mutex::new(TurnDiffTracker::default())),
|
||||||
|
call_id: "call-1".to_string(),
|
||||||
|
tool_name: REQUEST_USER_INPUT_TOOL_NAME.to_string(),
|
||||||
|
tool_namespace: None,
|
||||||
|
payload: ToolPayload::Function {
|
||||||
|
arguments: json!({
|
||||||
|
"questions": [{
|
||||||
|
"header": "Hdr",
|
||||||
|
"question": "Pick one",
|
||||||
|
"id": "pick_one",
|
||||||
|
"options": [
|
||||||
|
{
|
||||||
|
"label": "A",
|
||||||
|
"description": "A"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"label": "B",
|
||||||
|
"description": "B"
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}]
|
||||||
|
})
|
||||||
|
.to_string(),
|
||||||
|
},
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
|
||||||
|
let Err(err) = result else {
|
||||||
|
panic!("sub-agent request_user_input should fail");
|
||||||
|
};
|
||||||
|
assert_eq!(
|
||||||
|
err,
|
||||||
|
FunctionCallError::RespondToModel(
|
||||||
|
"request_user_input can only be used by the root thread".to_string(),
|
||||||
|
)
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -105,7 +105,6 @@ pub struct ToolsConfig {
|
|||||||
pub collab_tools: bool,
|
pub collab_tools: bool,
|
||||||
pub multi_agent_v2: bool,
|
pub multi_agent_v2: bool,
|
||||||
pub hide_spawn_agent_metadata: bool,
|
pub hide_spawn_agent_metadata: bool,
|
||||||
pub request_user_input: bool,
|
|
||||||
pub default_mode_request_user_input: bool,
|
pub default_mode_request_user_input: bool,
|
||||||
pub experimental_supported_tools: Vec<String>,
|
pub experimental_supported_tools: Vec<String>,
|
||||||
pub agent_jobs_tools: bool,
|
pub agent_jobs_tools: bool,
|
||||||
@@ -144,9 +143,8 @@ impl ToolsConfig {
|
|||||||
let include_multi_agent_v2 = features.enabled(Feature::MultiAgentV2);
|
let include_multi_agent_v2 = features.enabled(Feature::MultiAgentV2);
|
||||||
let hide_spawn_agent_metadata = features.enabled(Feature::DebugHideSpawnAgentMetadata);
|
let hide_spawn_agent_metadata = features.enabled(Feature::DebugHideSpawnAgentMetadata);
|
||||||
let include_agent_jobs = features.enabled(Feature::SpawnCsv);
|
let include_agent_jobs = features.enabled(Feature::SpawnCsv);
|
||||||
let include_request_user_input = !matches!(session_source, SessionSource::SubAgent(_));
|
|
||||||
let include_default_mode_request_user_input =
|
let include_default_mode_request_user_input =
|
||||||
include_request_user_input && features.enabled(Feature::DefaultModeRequestUserInput);
|
features.enabled(Feature::DefaultModeRequestUserInput);
|
||||||
let include_search_tool =
|
let include_search_tool =
|
||||||
model_info.supports_search_tool && features.enabled(Feature::ToolSearch);
|
model_info.supports_search_tool && features.enabled(Feature::ToolSearch);
|
||||||
let include_tool_suggest = features.enabled(Feature::ToolSuggest)
|
let include_tool_suggest = features.enabled(Feature::ToolSuggest)
|
||||||
@@ -222,7 +220,6 @@ impl ToolsConfig {
|
|||||||
collab_tools: include_collab_tools,
|
collab_tools: include_collab_tools,
|
||||||
multi_agent_v2: include_multi_agent_v2,
|
multi_agent_v2: include_multi_agent_v2,
|
||||||
hide_spawn_agent_metadata,
|
hide_spawn_agent_metadata,
|
||||||
request_user_input: include_request_user_input,
|
|
||||||
default_mode_request_user_input: include_default_mode_request_user_input,
|
default_mode_request_user_input: include_default_mode_request_user_input,
|
||||||
experimental_supported_tools: model_info.experimental_supported_tools.clone(),
|
experimental_supported_tools: model_info.experimental_supported_tools.clone(),
|
||||||
agent_jobs_tools: include_agent_jobs,
|
agent_jobs_tools: include_agent_jobs,
|
||||||
|
|||||||
@@ -131,9 +131,10 @@ fn shell_zsh_fork_prefers_shell_command_over_unified_exec() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn subagents_disable_request_user_input_and_agent_jobs_workers_opt_in_by_label() {
|
fn subagents_keep_request_user_input_mode_config_and_agent_jobs_workers_opt_in_by_label() {
|
||||||
let model_info = model_info();
|
let model_info = model_info();
|
||||||
let mut features = Features::with_defaults();
|
let mut features = Features::with_defaults();
|
||||||
|
features.enable(Feature::DefaultModeRequestUserInput);
|
||||||
features.enable(Feature::SpawnCsv);
|
features.enable(Feature::SpawnCsv);
|
||||||
|
|
||||||
let available_models = Vec::new();
|
let available_models = Vec::new();
|
||||||
@@ -149,8 +150,7 @@ fn subagents_disable_request_user_input_and_agent_jobs_workers_opt_in_by_label()
|
|||||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||||
});
|
});
|
||||||
|
|
||||||
assert!(!tools_config.request_user_input);
|
assert!(tools_config.default_mode_request_user_input);
|
||||||
assert!(!tools_config.default_mode_request_user_input);
|
|
||||||
assert!(tools_config.agent_jobs_tools);
|
assert!(tools_config.agent_jobs_tools);
|
||||||
assert!(tools_config.agent_jobs_worker_tools);
|
assert!(tools_config.agent_jobs_worker_tools);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -206,7 +206,6 @@ pub fn build_tool_registry_plan(
|
|||||||
plan.register_handler("js_repl_reset", ToolHandlerKind::JsReplReset);
|
plan.register_handler("js_repl_reset", ToolHandlerKind::JsReplReset);
|
||||||
}
|
}
|
||||||
|
|
||||||
if config.request_user_input {
|
|
||||||
plan.push_spec(
|
plan.push_spec(
|
||||||
create_request_user_input_tool(request_user_input_tool_description(
|
create_request_user_input_tool(request_user_input_tool_description(
|
||||||
config.default_mode_request_user_input,
|
config.default_mode_request_user_input,
|
||||||
@@ -218,7 +217,6 @@ pub fn build_tool_registry_plan(
|
|||||||
REQUEST_USER_INPUT_TOOL_NAME,
|
REQUEST_USER_INPUT_TOOL_NAME,
|
||||||
ToolHandlerKind::RequestUserInput,
|
ToolHandlerKind::RequestUserInput,
|
||||||
);
|
);
|
||||||
}
|
|
||||||
|
|
||||||
if config.request_permissions_tool_enabled {
|
if config.request_permissions_tool_enabled {
|
||||||
plan.push_spec(
|
plan.push_spec(
|
||||||
|
|||||||
@@ -534,9 +534,9 @@ fn test_build_specs_agent_job_worker_tools_enabled() {
|
|||||||
"close_agent",
|
"close_agent",
|
||||||
"spawn_agents_on_csv",
|
"spawn_agents_on_csv",
|
||||||
"report_agent_job_result",
|
"report_agent_job_result",
|
||||||
|
REQUEST_USER_INPUT_TOOL_NAME,
|
||||||
],
|
],
|
||||||
);
|
);
|
||||||
assert_lacks_tool_name(&tools, "request_user_input");
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
Reference in new issue
Block a user