Compare commits

..

1 Commits

Author SHA1 Message Date
Ahmed Ibrahim
811d686d69 Increase apply-patch test wait timeout 2026-04-07 00:07:46 -07:00
16 changed files with 66 additions and 190 deletions

View File

@@ -362,9 +362,6 @@
"connectors": {
"type": "boolean"
},
"debug_hide_spawn_agent_metadata": {
"type": "boolean"
},
"default_mode_request_user_input": {
"type": "boolean"
},
@@ -2075,9 +2072,6 @@
"connectors": {
"type": "boolean"
},
"debug_hide_spawn_agent_metadata": {
"type": "boolean"
},
"default_mode_request_user_input": {
"type": "boolean"
},

View File

@@ -64,12 +64,6 @@ async fn apply_role_to_config_inner(
return Ok(());
};
let role_layer_toml = load_role_layer_toml(config, config_file, is_built_in, role_name).await?;
if role_layer_toml
.as_table()
.is_some_and(toml::map::Map::is_empty)
{
return Ok(());
}
let (preserve_current_profile, preserve_current_provider) =
preservation_policy(config, &role_layer_toml);

View File

@@ -87,22 +87,6 @@ async fn apply_explorer_role_sets_model_and_adds_session_flags_layer() {
assert_eq!(session_flags_layer_count(&config), before_layers + 1);
}
#[tokio::test]
async fn apply_empty_explorer_role_preserves_current_model_and_reasoning_effort() {
let (_home, mut config) = test_config_with_cli_overrides(Vec::new()).await;
let before_layers = session_flags_layer_count(&config);
config.model = Some("gpt-5.4-mini".to_string());
config.model_reasoning_effort = Some(ReasoningEffort::High);
apply_role_to_config(&mut config, Some("explorer"))
.await
.expect("explorer role should apply");
assert_eq!(config.model.as_deref(), Some("gpt-5.4-mini"));
assert_eq!(config.model_reasoning_effort, Some(ReasoningEffort::High));
assert_eq!(session_flags_layer_count(&config), before_layers);
}
#[tokio::test]
async fn apply_role_returns_unavailable_for_missing_user_role_file() {
let (_home, mut config) = test_config_with_cli_overrides(Vec::new()).await;

View File

@@ -1554,7 +1554,7 @@ async fn multi_agent_v2_interrupted_turn_does_not_notify_parent() {
}
#[tokio::test]
async fn multi_agent_v2_spawn_omits_agent_id_when_named() {
async fn multi_agent_v2_spawn_includes_agent_id_key_when_named() {
let (mut session, mut turn) = make_session_and_context().await;
let manager = thread_manager();
let root = manager
@@ -1586,7 +1586,7 @@ async fn multi_agent_v2_spawn_omits_agent_id_when_named() {
let result: serde_json::Value =
serde_json::from_str(&content).expect("spawn_agent result should be json");
assert!(result.get("agent_id").is_none());
assert_eq!(result["agent_id"], serde_json::Value::Null);
assert_eq!(result["task_name"], "/root/test_process");
assert!(result.get("nickname").is_some());
assert_eq!(success, Some(true));

View File

@@ -209,6 +209,7 @@ impl ToolHandler for Handler {
})?;
Ok(SpawnAgentResult {
agent_id: None,
task_name,
nickname,
})
@@ -268,6 +269,7 @@ impl SpawnAgentArgs {
#[derive(Debug, Serialize)]
pub(crate) struct SpawnAgentResult {
agent_id: Option<String>,
task_name: String,
nickname: Option<String>,
}

View File

@@ -5,7 +5,6 @@ use crate::tools::context::ToolPayload;
use crate::tools::handlers::parse_arguments;
use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
use codex_protocol::protocol::SessionSource;
use codex_protocol::request_user_input::RequestUserInputArgs;
use codex_tools::REQUEST_USER_INPUT_TOOL_NAME;
use codex_tools::normalize_request_user_input_args;
@@ -40,12 +39,6 @@ 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;
if let Some(message) =
request_user_input_unavailable_message(mode, self.default_mode_request_user_input)
@@ -74,7 +67,3 @@ impl ToolHandler for RequestUserInputHandler {
Ok(FunctionToolOutput::from_text(content, Some(true)))
}
}
#[cfg(test)]
#[path = "request_user_input_tests.rs"]
mod tests;

View File

@@ -1,66 +0,0 @@
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(),
)
);
}

View File

@@ -582,12 +582,12 @@ impl UnifiedExecProcessManager {
pub(crate) async fn open_session_with_exec_env(
&self,
process_id: i32,
request: &ExecRequest,
env: &ExecRequest,
tty: bool,
mut spawn_lifecycle: SpawnLifecycleHandle,
environment: &codex_exec_server::Environment,
) -> Result<UnifiedExecProcess, UnifiedExecError> {
let (program, args) = request
let (program, args) = env
.command
.split_first()
.ok_or(UnifiedExecError::MissingCommandLine)?;
@@ -604,24 +604,24 @@ impl UnifiedExecProcessManager {
.get_exec_backend()
.start(codex_exec_server::ExecParams {
process_id: exec_server_process_id(process_id).into(),
argv: request.command.clone(),
cwd: request.cwd.clone(),
env: request.env.clone(),
argv: env.command.clone(),
cwd: env.cwd.clone(),
env: env.env.clone(),
tty,
arg0: request.arg0.clone(),
arg0: env.arg0.clone(),
})
.await
.map_err(|err| UnifiedExecError::create_process(err.to_string()))?;
return UnifiedExecProcess::from_remote_started(started, request.sandbox).await;
return UnifiedExecProcess::from_remote_started(started, env.sandbox).await;
}
let spawn_result = if tty {
codex_utils_pty::pty::spawn_process_with_inherited_fds(
program,
args,
request.cwd.as_path(),
&request.env,
&request.arg0,
env.cwd.as_path(),
&env.env,
&env.arg0,
codex_utils_pty::TerminalSize::default(),
&inherited_fds,
)
@@ -630,9 +630,9 @@ impl UnifiedExecProcessManager {
codex_utils_pty::pipe::spawn_process_no_stdin_with_inherited_fds(
program,
args,
request.cwd.as_path(),
&request.env,
&request.arg0,
env.cwd.as_path(),
&env.env,
&env.arg0,
&inherited_fds,
)
.await
@@ -640,7 +640,7 @@ impl UnifiedExecProcessManager {
let spawned =
spawn_result.map_err(|err| UnifiedExecError::create_process(err.to_string()))?;
spawn_lifecycle.after_spawn();
UnifiedExecProcess::from_spawned(spawned, request.sandbox, spawn_lifecycle).await
UnifiedExecProcess::from_spawned(spawned, env.sandbox, spawn_lifecycle).await
}
pub(super) async fn open_session_with_sandbox(

View File

@@ -49,8 +49,8 @@ use crate::responses::WebSocketTestServer;
use crate::responses::output_value_to_text;
use crate::responses::start_mock_server;
use crate::streaming_sse::StreamingSseServer;
use crate::wait_for_event;
use crate::wait_for_event_match;
use crate::wait_for_event_with_timeout;
use wiremock::Match;
use wiremock::matchers::path_regex;
@@ -767,10 +767,14 @@ impl TestCodex {
_ => None,
})
.await;
wait_for_event(&self.codex, |event| match event {
EventMsg::TurnComplete(event) => event.turn_id == turn_id,
_ => false,
})
wait_for_event_with_timeout(
&self.codex,
|event| match event {
EventMsg::TurnComplete(event) => event.turn_id == turn_id,
_ => false,
},
Duration::from_secs(60),
)
.await;
Ok(())
}

View File

@@ -138,8 +138,6 @@ pub enum Feature {
Collab,
/// Enable task-path-based multi-agent routing.
MultiAgentV2,
/// Hide spawn_agent agent/model override fields from the model-visible tool schema.
DebugHideSpawnAgentMetadata,
/// Enable CSV-backed agent job tools.
SpawnCsv,
/// Enable apps.
@@ -708,12 +706,6 @@ pub const FEATURES: &[FeatureSpec] = &[
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::DebugHideSpawnAgentMetadata,
key: "debug_hide_spawn_agent_metadata",
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::SpawnCsv,
key: "enable_fanout",

View File

@@ -10,7 +10,6 @@ use std::collections::BTreeMap;
pub struct SpawnAgentToolOptions<'a> {
pub available_models: &'a [ModelPreset],
pub agent_type_description: String,
pub hide_agent_type_model_reasoning: bool,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -21,19 +20,15 @@ pub struct WaitAgentTimeoutOptions {
}
pub fn create_spawn_agent_tool_v1(options: SpawnAgentToolOptions<'_>) -> ToolSpec {
let available_models_description = (!options.hide_agent_type_model_reasoning)
.then(|| spawn_agent_models_description(options.available_models));
let available_models_description = spawn_agent_models_description(options.available_models);
let return_value_description =
"Returns the spawned agent id plus the user-facing nickname when available.";
let mut properties = spawn_agent_common_properties_v1(&options.agent_type_description);
if options.hide_agent_type_model_reasoning {
hide_spawn_agent_metadata_options(&mut properties);
}
let properties = spawn_agent_common_properties_v1(&options.agent_type_description);
ToolSpec::Function(ResponsesApiTool {
name: "spawn_agent".to_string(),
description: spawn_agent_tool_description(
available_models_description.as_deref(),
&available_models_description,
return_value_description,
),
strict: false,
@@ -48,13 +43,9 @@ pub fn create_spawn_agent_tool_v1(options: SpawnAgentToolOptions<'_>) -> ToolSpe
}
pub fn create_spawn_agent_tool_v2(options: SpawnAgentToolOptions<'_>) -> ToolSpec {
let available_models_description = (!options.hide_agent_type_model_reasoning)
.then(|| spawn_agent_models_description(options.available_models));
let available_models_description = spawn_agent_models_description(options.available_models);
let return_value_description = "Returns the canonical task name for the spawned agent, plus the user-facing nickname when available.";
let mut properties = spawn_agent_common_properties_v2(&options.agent_type_description);
if options.hide_agent_type_model_reasoning {
hide_spawn_agent_metadata_options(&mut properties);
}
properties.insert(
"task_name".to_string(),
JsonSchema::String {
@@ -68,7 +59,7 @@ pub fn create_spawn_agent_tool_v2(options: SpawnAgentToolOptions<'_>) -> ToolSpe
ToolSpec::Function(ResponsesApiTool {
name: "spawn_agent".to_string(),
description: spawn_agent_tool_description(
available_models_description.as_deref(),
&available_models_description,
return_value_description,
),
strict: false,
@@ -375,6 +366,10 @@ fn spawn_agent_output_schema_v2() -> Value {
json!({
"type": "object",
"properties": {
"agent_id": {
"type": ["string", "null"],
"description": "Legacy thread identifier for the spawned agent."
},
"task_name": {
"type": "string",
"description": "Canonical task name for the spawned agent."
@@ -384,7 +379,7 @@ fn spawn_agent_output_schema_v2() -> Value {
"description": "User-facing nickname for the spawned agent when available."
}
},
"required": ["task_name", "nickname"],
"required": ["agent_id", "task_name", "nickname"],
"additionalProperties": false
})
}
@@ -642,30 +637,18 @@ fn spawn_agent_common_properties_v2(agent_type_description: &str) -> BTreeMap<St
])
}
fn hide_spawn_agent_metadata_options(properties: &mut BTreeMap<String, JsonSchema>) {
properties.remove("agent_type");
properties.remove("model");
properties.remove("reasoning_effort");
}
fn spawn_agent_tool_description(
available_models_description: Option<&str>,
available_models_description: &str,
return_value_description: &str,
) -> String {
let agent_role_guidance = available_models_description
.map(|description| {
format!(
"Agent-role guidance below only helps choose which agent to use after spawning is already authorized; it never authorizes spawning by itself.\n{description}"
)
})
.unwrap_or_default();
format!(
r#"
Only use `spawn_agent` if and only if the user explicitly asks for sub-agents, delegation, or parallel agent work.
Requests for depth, thoroughness, research, investigation, or detailed codebase analysis do not count as permission to spawn.
{agent_role_guidance}
Agent-role guidance below only helps choose which agent to use after spawning is already authorized; it never authorizes spawning by itself.
Spawn a sub-agent for a well-scoped task. {return_value_description} This spawn_agent tool provides you access to smaller but more efficient sub-agents. A mini model can solve many tasks faster than the main model. You should follow the rules and guidelines below to use this tool.
{available_models_description}
### When to delegate vs. do the subtask yourself
- First, quickly analyze the overall user task and form a succinct high-level plan. Identify which tasks are immediate blockers on the critical path, and which tasks are sidecar tasks that are needed but can run in parallel without blocking the next local step. As part of that plan, explicitly decide what immediate task you should do locally right now. Do this planning step before delegating to agents so you do not hand off the immediate blocking task to a submodel and then waste time waiting on it.
- Use the smaller subagent when a subtask is easy enough for it to handle and can run in parallel with your local work. Prefer delegating concrete, bounded sidecar tasks that materially advance the main task without blocking your immediate next local step.

View File

@@ -34,7 +34,6 @@ fn spawn_agent_tool_v2_requires_task_name_and_lists_visible_models() {
model_preset("hidden", /*show_in_picker*/ false),
],
agent_type_description: "role help".to_string(),
hide_agent_type_model_reasoning: false,
});
let ToolSpec::Function(ResponsesApiTool {
@@ -73,7 +72,7 @@ fn spawn_agent_tool_v2_requires_task_name_and_lists_visible_models() {
);
assert_eq!(
output_schema.expect("spawn_agent output schema")["required"],
json!(["task_name", "nickname"])
json!(["agent_id", "task_name", "nickname"])
);
}
@@ -82,7 +81,6 @@ fn spawn_agent_tool_v1_keeps_legacy_fork_context_field() {
let tool = create_spawn_agent_tool_v1(SpawnAgentToolOptions {
available_models: &[],
agent_type_description: "role help".to_string(),
hide_agent_type_model_reasoning: false,
});
let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = tool else {

View File

@@ -104,7 +104,7 @@ pub struct ToolsConfig {
pub can_request_original_image_detail: bool,
pub collab_tools: bool,
pub multi_agent_v2: bool,
pub hide_spawn_agent_metadata: bool,
pub request_user_input: bool,
pub default_mode_request_user_input: bool,
pub experimental_supported_tools: Vec<String>,
pub agent_jobs_tools: bool,
@@ -141,10 +141,10 @@ impl ToolsConfig {
include_js_repl && features.enabled(Feature::JsReplToolsOnly);
let include_collab_tools = features.enabled(Feature::Collab);
let include_multi_agent_v2 = features.enabled(Feature::MultiAgentV2);
let hide_spawn_agent_metadata = features.enabled(Feature::DebugHideSpawnAgentMetadata);
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 =
features.enabled(Feature::DefaultModeRequestUserInput);
include_request_user_input && features.enabled(Feature::DefaultModeRequestUserInput);
let include_search_tool =
model_info.supports_search_tool && features.enabled(Feature::ToolSearch);
let include_tool_suggest = features.enabled(Feature::ToolSuggest)
@@ -219,7 +219,7 @@ impl ToolsConfig {
can_request_original_image_detail: include_original_image_detail,
collab_tools: include_collab_tools,
multi_agent_v2: include_multi_agent_v2,
hide_spawn_agent_metadata,
request_user_input: include_request_user_input,
default_mode_request_user_input: include_default_mode_request_user_input,
experimental_supported_tools: model_info.experimental_supported_tools.clone(),
agent_jobs_tools: include_agent_jobs,

View File

@@ -131,10 +131,9 @@ fn shell_zsh_fork_prefers_shell_command_over_unified_exec() {
}
#[test]
fn subagents_keep_request_user_input_mode_config_and_agent_jobs_workers_opt_in_by_label() {
fn subagents_disable_request_user_input_and_agent_jobs_workers_opt_in_by_label() {
let model_info = model_info();
let mut features = Features::with_defaults();
features.enable(Feature::DefaultModeRequestUserInput);
features.enable(Feature::SpawnCsv);
let available_models = Vec::new();
@@ -150,7 +149,8 @@ fn subagents_keep_request_user_input_mode_config_and_agent_jobs_workers_opt_in_b
windows_sandbox_level: WindowsSandboxLevel::Disabled,
});
assert!(tools_config.default_mode_request_user_input);
assert!(!tools_config.request_user_input);
assert!(!tools_config.default_mode_request_user_input);
assert!(tools_config.agent_jobs_tools);
assert!(tools_config.agent_jobs_worker_tools);
}

View File

@@ -206,17 +206,19 @@ pub fn build_tool_registry_plan(
plan.register_handler("js_repl_reset", ToolHandlerKind::JsReplReset);
}
plan.push_spec(
create_request_user_input_tool(request_user_input_tool_description(
config.default_mode_request_user_input,
)),
/*supports_parallel_tool_calls*/ false,
config.code_mode_enabled,
);
plan.register_handler(
REQUEST_USER_INPUT_TOOL_NAME,
ToolHandlerKind::RequestUserInput,
);
if config.request_user_input {
plan.push_spec(
create_request_user_input_tool(request_user_input_tool_description(
config.default_mode_request_user_input,
)),
/*supports_parallel_tool_calls*/ false,
config.code_mode_enabled,
);
plan.register_handler(
REQUEST_USER_INPUT_TOOL_NAME,
ToolHandlerKind::RequestUserInput,
);
}
if config.request_permissions_tool_enabled {
plan.push_spec(
@@ -353,7 +355,6 @@ pub fn build_tool_registry_plan(
create_spawn_agent_tool_v2(SpawnAgentToolOptions {
available_models: &config.available_models,
agent_type_description,
hide_agent_type_model_reasoning: config.hide_spawn_agent_metadata,
}),
/*supports_parallel_tool_calls*/ false,
config.code_mode_enabled,
@@ -396,7 +397,6 @@ pub fn build_tool_registry_plan(
create_spawn_agent_tool_v1(SpawnAgentToolOptions {
available_models: &config.available_models,
agent_type_description,
hide_agent_type_model_reasoning: config.hide_spawn_agent_metadata,
}),
/*supports_parallel_tool_calls*/ false,
config.code_mode_enabled,

View File

@@ -243,7 +243,10 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() {
let output_schema = output_schema
.as_ref()
.expect("spawn_agent should define output schema");
assert_eq!(output_schema["required"], json!(["task_name", "nickname"]));
assert_eq!(
output_schema["required"],
json!(["agent_id", "task_name", "nickname"])
);
let send_message = find_tool(&tools, "send_message");
let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = &send_message.spec else {
@@ -522,9 +525,9 @@ fn test_build_specs_agent_job_worker_tools_enabled() {
"close_agent",
"spawn_agents_on_csv",
"report_agent_job_result",
REQUEST_USER_INPUT_TOOL_NAME,
],
);
assert_lacks_tool_name(&tools, "request_user_input");
}
#[test]
@@ -1853,7 +1856,6 @@ fn spawn_agent_tool_options(config: &ToolsConfig) -> SpawnAgentToolOptions<'_> {
SpawnAgentToolOptions {
available_models: &config.available_models,
agent_type_description: agent_type_description(config, DEFAULT_AGENT_TYPE_DESCRIPTION),
hide_agent_type_model_reasoning: config.hide_spawn_agent_metadata,
}
}